Skip to content

Commit

Permalink
Make IdentityInfo generic in pallet-identity (#1661)
Browse files Browse the repository at this point in the history 
Fixes #179 

# Description

This PR makes the structure containing identity information used in
`pallet-identity` generic through the pallet `Config`. Additionally, the
old structure is now available in a separate module called `simple`
(pending rename) and is compatible with the new interface.

Another change in this PR is that while the `additional` field in
`IdentityInfo` stays for backwards compatibility reasons, the associated
costs are stil present in the pallet through the `additional` function
in the `IdentityInformationProvider` interface. This function is marked
as deprecated as it is only a temporary solution to the backwards
compatibility problem we had. In short, we could have removed the
additional fields in the struct and done a migration, but we chose to
wait and do it off-chain through the genesis of the system parachain.
After we move the identity pallet to the parachain, additional fields
will be migrated into the existing fields and the `additional` key-value
store will be removed. Until that happens, this interface will provide
the necessary information to properly account for the associated costs.

Additionally, this PR fixes an unrelated issue; the `IdentityField` enum
used to represent the fields as bitflags couldn't store more than 8
fields, even though it was marked as `#[repr(u64)]`. This was because of
the `derive` implementation of `TypeInfo`, which assumed `u8` semantics.
The custom implementation of this trait in
0105cc0
fixes the issue.

---------

Signed-off-by: georgepisaltu <[email protected]>
Co-authored-by: Sam Johnson <[email protected]>
Co-authored-by: joe petrowski <[email protected]>
  • Loading branch information
@georgepisaltu @sam0x17 @joepetrowski
3 people authored Oct 24, 2023
1 parent 35eb133 commit 9185195
Show file tree
Hide file tree
Showing 10 changed files with 404 additions and 294 deletions.
2 changes: 2 additions & 0 deletions polkadot/runtime/rococo/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ use frame_support::{
};
use frame_system::EnsureRoot;
use pallet_grandpa::{fg_primitives, AuthorityId as GrandpaId};
use pallet_identity::simple::IdentityInfo;
use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
use pallet_session::historical as session_historical;
use pallet_transaction_payment::{CurrencyAdapter, FeeDetails, RuntimeDispatchInfo};
Expand Down Expand Up @@ -610,6 +611,7 @@ impl pallet_identity::Config for Runtime {
type SubAccountDeposit = SubAccountDeposit;
type MaxSubAccounts = MaxSubAccounts;
type MaxAdditionalFields = MaxAdditionalFields;
type IdentityInformation = IdentityInfo<MaxAdditionalFields>;
type MaxRegistrars = MaxRegistrars;
type Slashed = Treasury;
type ForceOrigin = EitherOf<EnsureRoot<Self::AccountId>, GeneralAdmin>;
Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/westend/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use frame_support::{
};
use frame_system::EnsureRoot;
use pallet_grandpa::{fg_primitives, AuthorityId as GrandpaId};
use pallet_identity::simple::IdentityInfo;
use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
use pallet_session::historical as session_historical;
use pallet_transaction_payment::{CurrencyAdapter, FeeDetails, RuntimeDispatchInfo};
Expand Down Expand Up @@ -876,6 +877,7 @@ impl pallet_identity::Config for Runtime {
type SubAccountDeposit = SubAccountDeposit;
type MaxSubAccounts = MaxSubAccounts;
type MaxAdditionalFields = MaxAdditionalFields;
type IdentityInformation = IdentityInfo<MaxAdditionalFields>;
type MaxRegistrars = MaxRegistrars;
type ForceOrigin = EitherOf<EnsureRoot<Self::AccountId>, GeneralAdmin>;
type RegistrarOrigin = EitherOf<EnsureRoot<Self::AccountId>, GeneralAdmin>;
Expand Down
2 changes: 2 additions & 0 deletions substrate/bin/node/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Moment, Nonce};
use pallet_asset_conversion::{NativeOrAssetId, NativeOrAssetIdConverter};
use pallet_broker::{CoreAssignment, CoreIndex, CoretimeInterface, PartsOf57600};
use pallet_election_provider_multi_phase::{GeometricDepositBase, SolutionAccuracyOf};
use pallet_identity::simple::IdentityInfo;
use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
use pallet_nfts::PalletFeatures;
use pallet_nis::WithMaximumOf;
Expand Down Expand Up @@ -1474,6 +1475,7 @@ impl pallet_identity::Config for Runtime {
type SubAccountDeposit = SubAccountDeposit;
type MaxSubAccounts = MaxSubAccounts;
type MaxAdditionalFields = MaxAdditionalFields;
type IdentityInformation = IdentityInfo<MaxAdditionalFields>;
type MaxRegistrars = MaxRegistrars;
type Slashed = Treasury;
type ForceOrigin = EnsureRootOrHalfCouncil;
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/alliance/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ use frame_support::{
},
weights::Weight,
};
use pallet_identity::IdentityField;
use pallet_identity::simple::IdentityField;
use scale_info::TypeInfo;

pub use pallet::*;
Expand Down
3 changes: 2 additions & 1 deletion substrate/frame/alliance/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub use frame_support::{
BoundedVec,
};
use frame_system::{EnsureRoot, EnsureSignedBy};
use pallet_identity::{Data, IdentityInfo, Judgement};
use pallet_identity::{simple::IdentityInfo, Data, Judgement};

pub use crate as pallet_alliance;

Expand Down Expand Up @@ -121,6 +121,7 @@ impl pallet_identity::Config for Test {
type SubAccountDeposit = SubAccountDeposit;
type MaxSubAccounts = MaxSubAccounts;
type MaxAdditionalFields = MaxAdditionalFields;
type IdentityInformation = IdentityInfo<MaxAdditionalFields>;
type MaxRegistrars = MaxRegistrars;
type Slashed = ();
type RegistrarOrigin = EnsureOneOrRoot;
Expand Down
60 changes: 18 additions & 42 deletions substrate/frame/identity/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use super::*;

use crate::Pallet as Identity;
use enumflags2::BitFlag;
use frame_benchmarking::{
account, impl_benchmark_test_suite, v2::*, whitelisted_caller, BenchmarkError,
};
Expand All @@ -48,14 +49,9 @@ fn add_registrars<T: Config>(r: u32) -> Result<(), &'static str> {
.expect("RegistrarOrigin has no successful origin required for the benchmark");
Identity::<T>::add_registrar(registrar_origin, registrar_lookup)?;
Identity::<T>::set_fee(RawOrigin::Signed(registrar.clone()).into(), i, 10u32.into())?;
let fields =
IdentityFields(
IdentityField::Display |
IdentityField::Legal | IdentityField::Web |
IdentityField::Riot | IdentityField::Email |
IdentityField::PgpFingerprint |
IdentityField::Image | IdentityField::Twitter,
);
let fields = IdentityFields(
<T::IdentityInformation as IdentityInformationProvider>::IdentityField::all(),
);
Identity::<T>::set_fields(RawOrigin::Signed(registrar.clone()).into(), i, fields)?;
}

Expand All @@ -81,7 +77,7 @@ fn create_sub_accounts<T: Config>(
// Set identity so `set_subs` does not fail.
if IdentityOf::<T>::get(who).is_none() {
let _ = T::Currency::make_free_balance_be(who, BalanceOf::<T>::max_value() / 2u32.into());
let info = create_identity_info::<T>(1);
let info = T::IdentityInformation::create_identity_info(1);
Identity::<T>::set_identity(who_origin.into(), Box::new(info))?;
}

Expand All @@ -102,24 +98,6 @@ fn add_sub_accounts<T: Config>(
Ok(subs)
}

// This creates an `IdentityInfo` object with `num_fields` extra fields.
// All data is pre-populated with some arbitrary bytes.
fn create_identity_info<T: Config>(num_fields: u32) -> IdentityInfo<T::MaxAdditionalFields> {
let data = Data::Raw(vec![0; 32].try_into().unwrap());

IdentityInfo {
additional: vec![(data.clone(), data.clone()); num_fields as usize].try_into().unwrap(),
display: data.clone(),
legal: data.clone(),
web: data.clone(),
riot: data.clone(),
email: data.clone(),
pgp_fingerprint: Some([0; 20]),
image: data.clone(),
twitter: data,
}
}

#[benchmarks]
mod benchmarks {
use super::*;
Expand Down Expand Up @@ -153,7 +131,7 @@ mod benchmarks {
let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());

// Add an initial identity
let initial_info = create_identity_info::<T>(1);
let initial_info = T::IdentityInformation::create_identity_info(1);
Identity::<T>::set_identity(caller_origin.clone(), Box::new(initial_info.clone()))?;

// User requests judgement from all the registrars, and they approve
Expand All @@ -174,7 +152,10 @@ mod benchmarks {
}

#[extrinsic_call]
_(RawOrigin::Signed(caller.clone()), Box::new(create_identity_info::<T>(x)));
_(
RawOrigin::Signed(caller.clone()),
Box::new(T::IdentityInformation::create_identity_info(x)),
);

assert_last_event::<T>(Event::<T>::IdentitySet { who: caller }.into());
Ok(())
Expand Down Expand Up @@ -235,7 +216,7 @@ mod benchmarks {
let _ = add_sub_accounts::<T>(&caller, s)?;

// Create their main identity with x additional fields
let info = create_identity_info::<T>(x);
let info = T::IdentityInformation::create_identity_info(x);
Identity::<T>::set_identity(caller_origin.clone(), Box::new(info.clone()))?;

// User requests judgement from all the registrars, and they approve
Expand Down Expand Up @@ -275,7 +256,7 @@ mod benchmarks {
add_registrars::<T>(r)?;

// Create their main identity with x additional fields
let info = create_identity_info::<T>(x);
let info = T::IdentityInformation::create_identity_info(x);
let caller_origin =
<T as frame_system::Config>::RuntimeOrigin::from(RawOrigin::Signed(caller.clone()));
Identity::<T>::set_identity(caller_origin.clone(), Box::new(info))?;
Expand All @@ -302,7 +283,7 @@ mod benchmarks {
add_registrars::<T>(r)?;

// Create their main identity with x additional fields
let info = create_identity_info::<T>(x);
let info = T::IdentityInformation::create_identity_info(x);
let caller_origin =
<T as frame_system::Config>::RuntimeOrigin::from(RawOrigin::Signed(caller.clone()));
Identity::<T>::set_identity(caller_origin.clone(), Box::new(info))?;
Expand Down Expand Up @@ -386,14 +367,9 @@ mod benchmarks {
.expect("RegistrarOrigin has no successful origin required for the benchmark");
Identity::<T>::add_registrar(registrar_origin, caller_lookup)?;

let fields =
IdentityFields(
IdentityField::Display |
IdentityField::Legal | IdentityField::Web |
IdentityField::Riot | IdentityField::Email |
IdentityField::PgpFingerprint |
IdentityField::Image | IdentityField::Twitter,
);
let fields = IdentityFields(
<T::IdentityInformation as IdentityInformationProvider>::IdentityField::all(),
);

let registrars = Registrars::<T>::get();
ensure!(
Expand Down Expand Up @@ -431,7 +407,7 @@ mod benchmarks {

add_registrars::<T>(r)?;

let info = create_identity_info::<T>(x);
let info = T::IdentityInformation::create_identity_info(x);
let info_hash = T::Hashing::hash_of(&info);
Identity::<T>::set_identity(user_origin.clone(), Box::new(info))?;

Expand Down Expand Up @@ -464,7 +440,7 @@ mod benchmarks {
let target_lookup = T::Lookup::unlookup(target.clone());
let _ = T::Currency::make_free_balance_be(&target, BalanceOf::<T>::max_value());

let info = create_identity_info::<T>(x);
let info = T::IdentityInformation::create_identity_info(x);
Identity::<T>::set_identity(target_origin.clone(), Box::new(info.clone()))?;
let _ = add_sub_accounts::<T>(&target, s)?;

Expand Down
Loading

0 comments on commit 9185195

Please sign in to comment.