Identity pallet improvements (#2048)

This PR is a follow up to #1661 - [x] rename the `simple` module to `legacy` - [x] fix benchmarks to disregard the number of additional fields - [x] change the storage deposits to charge per encoded byte of the identity information instance, removing the need for `fn additional(&self) -> usize` in `IdentityInformationProvider` - [x] ~add an extrinsic to rejig deposits to account for the change above~ - [ ] ~ensure through proper configuration that the new byte-based deposit is always lower than whatever is reserved now~ - [x] remove `IdentityFields` from the `set_fields` extrinsic signature, as per [this discussion](https://github.com/paritytech/polkadot-sdk/pull/1661#discussion_r1371703403) > ensure through proper configuration that the new byte-based deposit is always lower than whatever is reserved now Not sure this is needed anymore. If the new deposits are higher than what is currently on chain and users don't have enough funds to reserve what is needed, the extrinisc fails and they're basically grandfathered and frozen until they add more funds and/or make a change to their identity. This behavior seems fine to me. Original idea [here](https://github.com/paritytech/polkadot-sdk/pull/1661#issuecomment-1779606319). > add an extrinsic to rejig deposits to account for the change above This was initially implemented but now removed from this PR in favor of the implementation detailed [here](https://github.com/paritytech/polkadot-sdk/pull/2088). --------- Signed-off-by: georgepisaltu <george.pisaltu@parity.io> Co-authored-by: joepetrowski <joe@parity.io>
2026-04-30 11:57:56 +00:00 · 2023-11-03 21:38:26 +02:00
parent ca5f10567a
commit 21fbc00d04
15 changed files with 353 additions and 459 deletions
@@ -73,21 +73,23 @@
 #![cfg_attr(not(feature = "std"), no_std)]

 mod benchmarking;
-pub mod simple;
+pub mod legacy;
 #[cfg(test)]
 mod tests;
 mod types;
 pub mod weights;

-use frame_support::traits::{BalanceStatus, Currency, OnUnbalanced, ReservableCurrency};
+use frame_support::{
+	pallet_prelude::DispatchResult,
+	traits::{BalanceStatus, Currency, Get, OnUnbalanced, ReservableCurrency},
+};
 use sp_runtime::traits::{AppendZerosInput, Hash, Saturating, StaticLookup, Zero};
 use sp_std::prelude::*;
 pub use weights::WeightInfo;

 pub use pallet::*;
 pub use types::{
-	Data, IdentityFields, IdentityInformationProvider, Judgement, RegistrarIndex, RegistrarInfo,
-	Registration,
+	Data, IdentityInformationProvider, Judgement, RegistrarIndex, RegistrarInfo, Registration,
 };

 type BalanceOf<T> =
@@ -115,9 +117,9 @@ pub mod pallet {
 		#[pallet::constant]
 		type BasicDeposit: Get<BalanceOf<Self>>;

-		/// The amount held on deposit per additional field for a registered identity.
+		/// The amount held on deposit per encoded byte for a registered identity.
 		#[pallet::constant]
-		type FieldDeposit: Get<BalanceOf<Self>>;
+		type ByteDeposit: Get<BalanceOf<Self>>;

 		/// The amount held on deposit for a registered subaccount. This should account for the fact
 		/// that one storage item's value will increase by the size of an account ID, and there will
@@ -129,11 +131,6 @@ pub mod pallet {
 		#[pallet::constant]
 		type MaxSubAccounts: Get<u32>;

-		/// Maximum number of additional fields that may be stored in an ID. Needed to bound the I/O
-		/// required to access an identity, but can be pretty high.
-		#[pallet::constant]
-		type MaxAdditionalFields: Get<u32>;
-
 		/// Structure holding information about an identity.
 		type IdentityInformation: IdentityInformationProvider;

@@ -206,7 +203,7 @@ pub mod pallet {
 				RegistrarInfo<
 					BalanceOf<T>,
 					T::AccountId,
-					<T::IdentityInformation as IdentityInformationProvider>::IdentityField,
+					<T::IdentityInformation as IdentityInformationProvider>::FieldsIdentifier,
 				>,
 			>,
 			T::MaxRegistrars,
@@ -238,8 +235,6 @@ pub mod pallet {
 		InvalidIndex,
 		/// The target is invalid.
 		InvalidTarget,
-		/// Too many additional fields.
-		TooManyFields,
 		/// Maximum amount of registrars reached. Cannot add any more.
 		TooManyRegistrars,
 		/// Account ID is already named.
@@ -328,19 +323,15 @@ pub mod pallet {
 		///
 		/// Emits `IdentitySet` if successful.
 		#[pallet::call_index(1)]
-		#[pallet::weight(T::WeightInfo::set_identity(
-			T::MaxRegistrars::get(),
-			T::MaxAdditionalFields::get(),
-		))]
+		#[pallet::weight(T::WeightInfo::set_identity(T::MaxRegistrars::get()))]
 		pub fn set_identity(
 			origin: OriginFor<T>,
 			info: Box<T::IdentityInformation>,
 		) -> DispatchResultWithPostInfo {
 			let sender = ensure_signed(origin)?;
-			#[allow(deprecated)]
-			let extra_fields = info.additional() as u32;
-			ensure!(extra_fields <= T::MaxAdditionalFields::get(), Error::<T>::TooManyFields);
-			let fd = <BalanceOf<T>>::from(extra_fields) * T::FieldDeposit::get();
+			let encoded_byte_size = info.encoded_size() as u32;
+			let byte_deposit =
+				T::ByteDeposit::get().saturating_mul(<BalanceOf<T>>::from(encoded_byte_size));

 			let mut id = match <IdentityOf<T>>::get(&sender) {
 				Some(mut id) => {
@@ -357,7 +348,7 @@ pub mod pallet {
 			};

 			let old_deposit = id.deposit;
-			id.deposit = T::BasicDeposit::get() + fd;
+			id.deposit = T::BasicDeposit::get().saturating_add(byte_deposit);
 			if id.deposit > old_deposit {
 				T::Currency::reserve(&sender, id.deposit - old_deposit)?;
 			}
@@ -370,7 +361,7 @@ pub mod pallet {
 			<IdentityOf<T>>::insert(&sender, id);
 			Self::deposit_event(Event::IdentitySet { who: sender });

-			Ok(Some(T::WeightInfo::set_identity(judgements as u32, extra_fields)).into())
+			Ok(Some(T::WeightInfo::set_identity(judgements as u32)).into())
 		}

 		/// Set the sub-accounts of the sender.
@@ -404,7 +395,8 @@ pub mod pallet {
 			);

 			let (old_deposit, old_ids) = <SubsOf<T>>::get(&sender);
-			let new_deposit = T::SubAccountDeposit::get() * <BalanceOf<T>>::from(subs.len() as u32);
+			let new_deposit =
+				T::SubAccountDeposit::get().saturating_mul(<BalanceOf<T>>::from(subs.len() as u32));

 			let not_other_sub =
 				subs.iter().filter_map(|i| SuperOf::<T>::get(&i.0)).all(|i| i.0 == sender);
@@ -454,14 +446,13 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::clear_identity(
 			T::MaxRegistrars::get(),
 			T::MaxSubAccounts::get(),
-			T::MaxAdditionalFields::get(),
 		))]
 		pub fn clear_identity(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
 			let sender = ensure_signed(origin)?;

 			let (subs_deposit, sub_ids) = <SubsOf<T>>::take(&sender);
 			let id = <IdentityOf<T>>::take(&sender).ok_or(Error::<T>::NotNamed)?;
-			let deposit = id.total_deposit() + subs_deposit;
+			let deposit = id.total_deposit().saturating_add(subs_deposit);
 			for sub in sub_ids.iter() {
 				<SuperOf<T>>::remove(sub);
 			}
@@ -475,7 +466,6 @@ pub mod pallet {
 			Ok(Some(T::WeightInfo::clear_identity(
 				id.judgements.len() as u32,
 				sub_ids.len() as u32,
-				id.info.additional() as u32,
 			))
 			.into())
 		}
@@ -497,10 +487,7 @@ pub mod pallet {
 		///
 		/// Emits `JudgementRequested` if successful.
 		#[pallet::call_index(4)]
-		#[pallet::weight(T::WeightInfo::request_judgement(
-			T::MaxRegistrars::get(),
-			T::MaxAdditionalFields::get(),
-		))]
+		#[pallet::weight(T::WeightInfo::request_judgement(T::MaxRegistrars::get(),))]
 		pub fn request_judgement(
 			origin: OriginFor<T>,
 			#[pallet::compact] reg_index: RegistrarIndex,
@@ -530,8 +517,6 @@ pub mod pallet {
 			T::Currency::reserve(&sender, registrar.fee)?;

 			let judgements = id.judgements.len();
-			#[allow(deprecated)]
-			let extra_fields = id.info.additional();
 			<IdentityOf<T>>::insert(&sender, id);

 			Self::deposit_event(Event::JudgementRequested {
@@ -539,8 +524,7 @@ pub mod pallet {
 				registrar_index: reg_index,
 			});

-			Ok(Some(T::WeightInfo::request_judgement(judgements as u32, extra_fields as u32))
-				.into())
+			Ok(Some(T::WeightInfo::request_judgement(judgements as u32)).into())
 		}

 		/// Cancel a previous request.
@@ -554,10 +538,7 @@ pub mod pallet {
 		///
 		/// Emits `JudgementUnrequested` if successful.
 		#[pallet::call_index(5)]
-		#[pallet::weight(T::WeightInfo::cancel_request(
-			T::MaxRegistrars::get(),
-			T::MaxAdditionalFields::get(),
-		))]
+		#[pallet::weight(T::WeightInfo::cancel_request(T::MaxRegistrars::get()))]
 		pub fn cancel_request(
 			origin: OriginFor<T>,
 			reg_index: RegistrarIndex,
@@ -578,8 +559,6 @@ pub mod pallet {
 			let err_amount = T::Currency::unreserve(&sender, fee);
 			debug_assert!(err_amount.is_zero());
 			let judgements = id.judgements.len();
-			#[allow(deprecated)]
-			let extra_fields = id.info.additional();
 			<IdentityOf<T>>::insert(&sender, id);

 			Self::deposit_event(Event::JudgementUnrequested {
@@ -587,7 +566,7 @@ pub mod pallet {
 				registrar_index: reg_index,
 			});

-			Ok(Some(T::WeightInfo::cancel_request(judgements as u32, extra_fields as u32)).into())
+			Ok(Some(T::WeightInfo::cancel_request(judgements as u32)).into())
 		}

 		/// Set the fee required for a judgement to be requested from a registrar.
@@ -669,26 +648,21 @@ pub mod pallet {
 		pub fn set_fields(
 			origin: OriginFor<T>,
 			#[pallet::compact] index: RegistrarIndex,
-			fields: IdentityFields<
-				<T::IdentityInformation as IdentityInformationProvider>::IdentityField,
-			>,
+			fields: <T::IdentityInformation as IdentityInformationProvider>::FieldsIdentifier,
 		) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;

-			let registrars = <Registrars<T>>::mutate(|rs| -> Result<usize, DispatchError> {
-				rs.get_mut(index as usize)
-					.and_then(|x| x.as_mut())
-					.and_then(|r| {
-						if r.account == who {
-							r.fields = fields;
-							Some(())
-						} else {
-							None
-						}
-					})
-					.ok_or_else(|| DispatchError::from(Error::<T>::InvalidIndex))?;
-				Ok(rs.len())
-			})?;
+			let registrars =
+				<Registrars<T>>::mutate(|registrars| -> Result<usize, DispatchError> {
+					let registrar = registrars
+						.get_mut(index as usize)
+						.and_then(|r| r.as_mut())
+						.filter(|r| r.account == who)
+						.ok_or_else(|| DispatchError::from(Error::<T>::InvalidIndex))?;
+					registrar.fields = fields;
+
+					Ok(registrars.len())
+				})?;
 			Ok(Some(T::WeightInfo::set_fields(registrars as u32)).into())
 		}

@@ -706,10 +680,7 @@ pub mod pallet {
 		///
 		/// Emits `JudgementGiven` if successful.
 		#[pallet::call_index(9)]
-		#[pallet::weight(T::WeightInfo::provide_judgement(
-			T::MaxRegistrars::get(),
-			T::MaxAdditionalFields::get(),
-		))]
+		#[pallet::weight(T::WeightInfo::provide_judgement(T::MaxRegistrars::get()))]
 		pub fn provide_judgement(
 			origin: OriginFor<T>,
 			#[pallet::compact] reg_index: RegistrarIndex,
@@ -752,13 +723,10 @@ pub mod pallet {
 			}

 			let judgements = id.judgements.len();
-			#[allow(deprecated)]
-			let extra_fields = id.info.additional();
 			<IdentityOf<T>>::insert(&target, id);
 			Self::deposit_event(Event::JudgementGiven { target, registrar_index: reg_index });

-			Ok(Some(T::WeightInfo::provide_judgement(judgements as u32, extra_fields as u32))
-				.into())
+			Ok(Some(T::WeightInfo::provide_judgement(judgements as u32)).into())
 		}

 		/// Remove an account's identity and sub-account information and slash the deposits.
@@ -777,7 +745,6 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::kill_identity(
 			T::MaxRegistrars::get(),
 			T::MaxSubAccounts::get(),
-			T::MaxAdditionalFields::get(),
 		))]
 		pub fn kill_identity(
 			origin: OriginFor<T>,
@@ -790,7 +757,7 @@ pub mod pallet {
 			// Grab their deposit (and check that they have one).
 			let (subs_deposit, sub_ids) = <SubsOf<T>>::take(&target);
 			let id = <IdentityOf<T>>::take(&target).ok_or(Error::<T>::NotNamed)?;
-			let deposit = id.total_deposit() + subs_deposit;
+			let deposit = id.total_deposit().saturating_add(subs_deposit);
 			for sub in sub_ids.iter() {
 				<SuperOf<T>>::remove(sub);
 			}
@@ -800,12 +767,8 @@ pub mod pallet {
 			Self::deposit_event(Event::IdentityKilled { who: target, deposit });

 			#[allow(deprecated)]
-			Ok(Some(T::WeightInfo::kill_identity(
-				id.judgements.len() as u32,
-				sub_ids.len() as u32,
-				id.info.additional() as u32,
-			))
-			.into())
+			Ok(Some(T::WeightInfo::kill_identity(id.judgements.len() as u32, sub_ids.len() as u32))
+				.into())
 		}

 		/// Add the given account to the sender's subs.
@@ -936,7 +899,10 @@ impl<T: Config> Pallet<T> {
 	}

 	/// Check if the account has corresponding identity information by the identity field.
-	pub fn has_identity(who: &T::AccountId, fields: u64) -> bool {
+	pub fn has_identity(
+		who: &T::AccountId,
+		fields: <T::IdentityInformation as IdentityInformationProvider>::FieldsIdentifier,
+	) -> bool {
 		IdentityOf::<T>::get(who)
 			.map_or(false, |registration| (registration.info.has_identity(fields)))
 	}