Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Add GenesisConfig to identity pallet #14774

Open
grw-ms wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add GenesisConfig to identity pallet #14774

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3a6de3a
feat: stub IdentityConfig
grw-ms Aug 8, 2023
55ba65c
feat: register identities in GenesisBuild
grw-ms Aug 9, 2023
232fdf0
use BuildGenesisConfig instead of deprecated GenesisConfig<T>
grw-ms Aug 16, 2023
6a03dc6
use String::as_bytes().to_vec() instead of encode(), add test
grw-ms Aug 16, 2023
587a6fc
take Vec<u8> instead of String
grw-ms Aug 16, 2023
89774f9
remove (feature="std") gate
grw-ms Aug 17, 2023
b52f93e
take (AccountId, BoundedVec) for GenesisConfig
grw-ms Aug 17, 2023
b5eae62
update test genesisconfig
grw-ms Aug 17, 2023
d35a8d4
pallet_identity: accept registrars in GenesisConfig not only identities
grw-ms Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions bin/node/cli/src/chain_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
use grandpa_primitives::AuthorityId as GrandpaId;
use kitchensink_runtime::{
constants::currency::*, wasm_binary_unwrap, BabeConfig, BalancesConfig, Block, CouncilConfig,
DemocracyConfig, ElectionsConfig, ImOnlineConfig, IndicesConfig, MaxNominations,
NominationPoolsConfig, SessionConfig, SessionKeys, SocietyConfig, StakerStatus, StakingConfig,
SudoConfig, SystemConfig, TechnicalCommitteeConfig,
DemocracyConfig, ElectionsConfig, IdentityConfig, ImOnlineConfig, IndicesConfig,
MaxNominations, NominationPoolsConfig, SessionConfig, SessionKeys, SocietyConfig, StakerStatus,
StakingConfig, SudoConfig, SystemConfig, TechnicalCommitteeConfig,
};
use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
use sc_chain_spec::ChainSpecExtension;
Expand Down Expand Up @@ -298,6 +298,7 @@ pub fn testnet_genesis(
balances: BalancesConfig {
balances: endowed_accounts.iter().cloned().map(|x| (x, ENDOWMENT)).collect(),
},
identity: IdentityConfig { identities: vec![] },
indices: IndicesConfig { indices: vec![] },
session: SessionConfig {
keys: initial_authorities
Expand Down
7 changes: 5 additions & 2 deletions bin/node/testing/src/genesis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
use crate::keyring::*;
use kitchensink_runtime::{
constants::currency::*, wasm_binary_unwrap, AccountId, AssetsConfig, BabeConfig,
BalancesConfig, GluttonConfig, GrandpaConfig, IndicesConfig, RuntimeGenesisConfig,
SessionConfig, SocietyConfig, StakerStatus, StakingConfig, SystemConfig,
BalancesConfig, GluttonConfig, GrandpaConfig, IdentityConfig, IndicesConfig,
RuntimeGenesisConfig, SessionConfig, SocietyConfig, StakerStatus, StakingConfig, SystemConfig,
BABE_GENESIS_EPOCH_CONFIG,
};
use sp_keyring::{Ed25519Keyring, Sr25519Keyring};
Expand Down Expand Up @@ -52,6 +52,9 @@ pub fn config_endowed(code: Option<&[u8]>, extra_endowed: Vec<AccountId>) -> Run
code: code.map(|x| x.to_vec()).unwrap_or_else(|| wasm_binary_unwrap().to_vec()),
..Default::default()
},
identity: IdentityConfig {
identities: vec![(alice(), "Alice".to_string()), (bob(), "Bob".to_string())],
Copy link
Contributor

@liamaharon liamaharon Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be mindful that Data in IdentityInfo is truncated to 32 bytes (see docs), so it is recommended to pass a hash of the data rather than the data itself for stuff like display.

},
indices: IndicesConfig { indices: vec![] },
balances: BalancesConfig { balances: endowed },
session: SessionConfig {
Expand Down
39 changes: 39 additions & 0 deletions frame/identity/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ mod types;
pub mod weights;

use frame_support::traits::{BalanceStatus, Currency, OnUnbalanced, ReservableCurrency};
use scale_info::prelude::string::String;
use sp_runtime::traits::{AppendZerosInput, Hash, Saturating, StaticLookup, Zero};
use sp_std::prelude::*;
pub use weights::WeightInfo;
Expand Down Expand Up @@ -201,6 +202,44 @@ pub mod pallet {
ValueQuery,
>;

#[pallet::genesis_config]
pub struct GenesisConfig<T: Config> {
pub identities: Vec<(T::AccountId, String)>,
Copy link
Contributor

@liamaharon liamaharon Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want this

Suggested change
pub identities: Vec<(T::AccountId, String)>,
pub identities: Vec<(T::AccountId, IdentityInfo)>,

Copy link
Member

@ggwpez ggwpez Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this was done to not require trait bounds on the IdentityInfo type?

Copy link
Contributor

@KiChjang KiChjang Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even so, it seems much more preferable to be able to specify all other identity-related fields in genesis config, so let's figure out what the trait bounds should be here...

Oh, it's just T::MaxAdditionalFields, so we should just use that:

Suggested change
pub identities: Vec<(T::AccountId, String)>,
pub identities: Vec<(T::AccountId, IdentityInfo<T::MaxAdditionalFields>)>,

Copy link
Author

@grw-ms grw-ms Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for comments both, I didn't use IdentityInfo initially as it wanted Deserialize<'_>, I agree it is preferable to support this though, I will try to add serde::[Des|S]erialize on these types

Copy link
Member

@ggwpez ggwpez Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what i meant with trait bounds. Normally we just use POD types here to not having to use Serde from within the runtime.
I think its fine.

Copy link
Member

@ggwpez ggwpez Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Vec<u8> would be a bit less biased than a String - otherwise the encoding is not fixed.

Copy link
Contributor

@KiChjang KiChjang Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll probably need BoundedVec then, no reason to not make this derive MEL.

Copy link
Author

@grw-ms grw-ms Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx, uses BoundedVec now

}

#[cfg(feature = "std")]
impl<T: Config> Default for GenesisConfig<T> {
fn default() -> Self {
GenesisConfig { identities: Default::default() }
}
}

#[pallet::genesis_build]
impl<T: Config> GenesisBuild<T> for GenesisConfig<T> {
fn build(&self) {
for (account, name) in &self.identities {
<IdentityOf<T>>::insert(
account,
Registration {
info: IdentityInfo {
display: Data::Raw(BoundedVec::try_from(name.encode()).unwrap()),
twitter: Data::None,
riot: Data::None,
email: Data::None,
pgp_fingerprint: None,
image: Data::None,
legal: Data::None,
web: Data::None,
additional: BoundedVec::default(),
},
judgements: BoundedVec::default(),
Copy link
Member

@ggwpez ggwpez Aug 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much sense does it make not to provide any judgments?
Having no judgement is as good as having no identity at all.
But not sure if its possible without also adding a genesis judge…

Copy link
Contributor

@KiChjang KiChjang Aug 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you suggested it, we are now depending upon you to implement a genesis identity judge :)

deposit: Zero::zero(),
},
);
}
}
}

#[pallet::error]
pub enum Error<T> {
/// Too many subs-accounts.
Expand Down