From 6bb2f4b93aafa4bbc408d10e0ed444f37db56151 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 00:26:29 -0700 Subject: [PATCH 01/18] Ensure data size of identity pallet is bounded --- Cargo.lock | 1 + frame/identity/Cargo.toml | 2 + frame/identity/src/lib.rs | 122 ++++++++++++++++++----- frame/identity/src/tests.rs | 4 +- frame/support/src/storage/bounded_vec.rs | 8 ++ 5 files changed, 112 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a33cb02f7f0d4..7bcf713397c90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5129,6 +5129,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "max-encoded-len", "pallet-balances", "parity-scale-codec", "sp-core", diff --git a/frame/identity/Cargo.toml b/frame/identity/Cargo.toml index fce79c56f80a9..0438183612b10 100644 --- a/frame/identity/Cargo.toml +++ b/frame/identity/Cargo.toml @@ -21,6 +21,7 @@ sp-runtime = { version = "3.0.0", default-features = false, path = "../../primit frame-benchmarking = { version = "3.1.0", default-features = false, path = "../benchmarking", optional = true } frame-support = { version = "3.0.0", default-features = false, path = "../support" } frame-system = { version = "3.0.0", default-features = false, path = "../system" } +max-encoded-len = { version = "3.0.0", default-features = false, path = "../../max-encoded-len", features = ["derive"] } [dev-dependencies] sp-core = { version = "3.0.0", path = "../../primitives/core" } @@ -36,6 +37,7 @@ std = [ "frame-benchmarking/std", "frame-support/std", "frame-system/std", + "max-encoded-len/std", ] runtime-benchmarks = ["frame-benchmarking"] try-runtime = ["frame-support/try-runtime"] diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index b71b069ccb74f..e96fa1fe02f59 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -78,12 +78,16 @@ mod benchmarking; pub mod weights; use sp_std::prelude::*; -use sp_std::{fmt::Debug, ops::Add, iter::once}; +use sp_std::{convert::TryInto, fmt::{self, Debug}, ops::Add, iter::once}; use enumflags2::BitFlags; use codec::{Encode, Decode}; use sp_runtime::RuntimeDebug; use sp_runtime::traits::{StaticLookup, Zero, AppendZerosInput, Saturating}; -use frame_support::traits::{Currency, ReservableCurrency, OnUnbalanced, BalanceStatus}; +use frame_support::{ + parameter_types, + traits::{BalanceStatus, Currency, Get, MaxEncodedLen, OnUnbalanced, ReservableCurrency}, + BoundedVec, +}; pub use weights::WeightInfo; pub use pallet::*; @@ -91,16 +95,20 @@ pub use pallet::*; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; +parameter_types! { + pub const MaxDataSize: u32 = 32; +} + /// Either underlying data blob if it is at most 32 bytes, or a hash of it. If the data is greater /// than 32-bytes then it will be truncated when encoding. /// /// Can also be `None`. -#[derive(Clone, Eq, PartialEq, RuntimeDebug)] +#[derive(Clone, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] pub enum Data { /// No data here. None, /// The data is stored directly. - Raw(Vec), + Raw(BoundedVec), /// Only the Blake2 hash of the data is stored. The preimage of the hash may be retrieved /// through some hash-lookup service. BlakeTwo256([u8; 32]), @@ -121,7 +129,9 @@ impl Decode for Data { Ok(match b { 0 => Data::None, n @ 1 ..= 33 => { - let mut r = vec![0u8; n as usize - 1]; + let mut r: BoundedVec<_, _> = vec![0u8; n as usize - 1] + .try_into() + .expect("bound checked in match arm condition; qed"); input.read(&mut r[..])?; Data::Raw(r) } @@ -166,7 +176,7 @@ pub type RegistrarIndex = u32; /// /// NOTE: Registrars may pay little attention to some fields. Registrars may want to make clear /// which fields their attestation is relevant for by off-chain means. -#[derive(Copy, Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug)] +#[derive(Copy, Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] pub enum Judgement< Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq > { @@ -250,12 +260,12 @@ impl Decode for IdentityFields { /// /// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra /// fields in a backwards compatible way through a specialized `Decode` impl. -#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug)] +#[derive(Encode, Decode, Eq, MaxEncodedLen)] #[cfg_attr(test, derive(Default))] -pub struct IdentityInfo { +pub struct IdentityInfo> { /// Additional fields of the identity that are not catered for with the struct's explicit /// fields. - pub additional: Vec<(Data, Data)>, + pub additional: BoundedVec<(Data, Data), FieldLimit>, /// A reasonable display name for the controller of the account. This should be whatever it is /// that it is typically known as and should not be confusable with other entities, given @@ -298,28 +308,85 @@ pub struct IdentityInfo { pub twitter: Data, } +#[cfg(not(feature = "std"))] +impl> Debug for IdentityInfo { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "") + } +} + +#[cfg(feature = "std")] +impl> Debug for IdentityInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("IdentityInfo") + .field("additional", &self.additional) + .field("display", &self.display) + .field("legal", &self.legal) + .field("web", &self.web) + .field("riot", &self.riot) + .field("email", &self.email) + .field("pgp_fingerprint", &self.pgp_fingerprint) + .field("image", &self.image) + .field("twitter", &self.twitter) + .finish() + } +} + +impl> PartialEq for IdentityInfo { + fn eq(&self, other: &IdentityInfo) -> bool { + self.additional == other.additional + && self.display == other.display + && self.legal == other.legal + && self.web == other.web + && self.riot == other.riot + && self.email == other.email + && self.image == other.image + && self.twitter == other.twitter + } +} + +impl> Clone for IdentityInfo { + fn clone(&self) -> Self { + Self { + additional: self.additional.clone(), + display: self.display.clone(), + legal: self.legal.clone(), + web: self.web.clone(), + riot: self.riot.clone(), + email: self.email.clone(), + pgp_fingerprint: self.pgp_fingerprint.clone(), + image: self.image.clone(), + twitter: self.twitter.clone(), + } + } +} + /// Information concerning the identity of the controller of an account. /// /// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a /// backwards compatible way through a specialized `Decode` impl. -#[derive(Clone, Encode, Eq, PartialEq, RuntimeDebug)] +#[derive(Clone, Encode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] pub struct Registration< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, > { /// Judgements from the registrars on this identity. Stored ordered by `RegistrarIndex`. There /// may be only a single judgement from each registrar. - pub judgements: Vec<(RegistrarIndex, Judgement)>, + pub judgements: BoundedVec<(RegistrarIndex, Judgement), MaxJudgments>, /// Amount held on deposit for this information. pub deposit: Balance, /// Information on the identity. - pub info: IdentityInfo, + pub info: IdentityInfo, } impl < Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, -> Registration { + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Registration { fn total_deposit(&self) -> Balance { self.deposit + self.judgements.iter() .map(|(_, ref j)| if let Judgement::FeePaid(fee) = j { *fee } else { Zero::zero() }) @@ -329,7 +396,9 @@ impl < impl< Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, -> Decode for Registration { + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Decode for Registration { fn decode(input: &mut I) -> sp_std::result::Result { let (judgements, deposit, info) = Decode::decode(&mut AppendZerosInput::new(input))?; Ok(Self { judgements, deposit, info }) @@ -422,7 +491,7 @@ pub mod pallet { _, Twox64Concat, T::AccountId, - Registration>, + Registration, T::MaxRegistrars, T::MaxAdditionalFields>, OptionQuery, >; @@ -461,7 +530,7 @@ pub mod pallet { #[pallet::getter(fn registrars)] pub(super) type Registrars = StorageValue< _, - Vec, T::AccountId>>>, + BoundedVec, T::AccountId>>, T::MaxRegistrars>, ValueQuery, >; @@ -555,9 +624,10 @@ pub mod pallet { let (i, registrar_count) = >::try_mutate( |registrars| -> Result<(RegistrarIndex, usize), DispatchError> { ensure!(registrars.len() < T::MaxRegistrars::get() as usize, Error::::TooManyRegistrars); - registrars.push(Some(RegistrarInfo { + registrars.try_push(Some(RegistrarInfo { account, fee: Zero::zero(), fields: Default::default() - })); + })) + .map_err(|_| Error::::TooManyRegistrars)?; Ok(((registrars.len() - 1) as RegistrarIndex, registrars.len())) } )?; @@ -590,7 +660,7 @@ pub mod pallet { T::MaxRegistrars::get().into(), // R T::MaxAdditionalFields::get().into(), // X ))] - pub fn set_identity(origin: OriginFor, info: IdentityInfo) -> DispatchResultWithPostInfo { + pub fn set_identity(origin: OriginFor, info: IdentityInfo) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; let extra_fields = info.additional.len() as u32; ensure!(extra_fields <= T::MaxAdditionalFields::get(), Error::::TooManyFields); @@ -603,7 +673,7 @@ pub mod pallet { id.info = info; id } - None => Registration { info, judgements: Vec::new(), deposit: Zero::zero() }, + None => Registration { info, judgements: BoundedVec::default(), deposit: Zero::zero() }, }; let old_deposit = id.deposit; @@ -786,7 +856,10 @@ pub mod pallet { } else { id.judgements[i] = item }, - Err(i) => id.judgements.insert(i, item), + Err(i) => id + .judgements + .try_insert(i, item) + .map_err(|_| Error::::TooManyRegistrars)?, } T::Currency::reserve(&sender, registrar.fee)?; @@ -988,7 +1061,10 @@ pub mod pallet { } id.judgements[position] = item } - Err(position) => id.judgements.insert(position, item), + Err(position) => id + .judgements + .try_insert(position, item) + .map_err(|_| Error::::TooManyRegistrars)?, } let judgements = id.judgements.len(); diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index 262b3211b6d1b..c33a2b37be106 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -139,7 +139,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { t.into() } -fn ten() -> IdentityInfo { +fn ten() -> IdentityInfo { IdentityInfo { display: Data::Raw(b"ten".to_vec()), legal: Data::Raw(b"The Right Ordinal Ten, Esq.".to_vec()), @@ -147,7 +147,7 @@ fn ten() -> IdentityInfo { } } -fn twenty() -> IdentityInfo { +fn twenty() -> IdentityInfo { IdentityInfo { display: Data::Raw(b"twenty".to_vec()), legal: Data::Raw(b"The Right Ordinal Twenty, Esq.".to_vec()), diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index d1c042b5db17f..ced4f94c0b347 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -122,6 +122,14 @@ impl BoundedVec { pub fn retain bool>(&mut self, f: F) { self.0.retain(f) } + + /// Exactly the same semantics as [`Vec::get_mut`]. + pub fn get_mut>( + &mut self, + index: I, + ) -> Option<&mut >::Output> { + self.0.get_mut(index) + } } impl> From> for Vec { From 228c9390b51e56dd2405e3b3319da2eed362beac Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 01:41:34 -0700 Subject: [PATCH 02/18] Fix unit tests for identity pallet --- frame/identity/src/lib.rs | 74 +++++++++++++++++++++++++++++++++++-- frame/identity/src/tests.rs | 56 ++++++++++++++-------------- 2 files changed, 98 insertions(+), 32 deletions(-) diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index e96fa1fe02f59..a4bd3f05831e1 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -261,7 +261,6 @@ impl Decode for IdentityFields { /// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra /// fields in a backwards compatible way through a specialized `Decode` impl. #[derive(Encode, Decode, Eq, MaxEncodedLen)] -#[cfg_attr(test, derive(Default))] pub struct IdentityInfo> { /// Additional fields of the identity that are not catered for with the struct's explicit /// fields. @@ -308,6 +307,23 @@ pub struct IdentityInfo> { pub twitter: Data, } +#[cfg(test)] +impl> Default for IdentityInfo { + fn default() -> Self { + Self { + additional: Default::default(), + display: Default::default(), + legal: Default::default(), + web: Default::default(), + riot: Default::default(), + email: Default::default(), + pgp_fingerprint: Default::default(), + image: Default::default(), + twitter: Default::default(), + } + } +} + #[cfg(not(feature = "std"))] impl> Debug for IdentityInfo { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -340,6 +356,7 @@ impl> PartialEq for IdentityInfo { && self.web == other.web && self.riot == other.riot && self.email == other.email + && self.pgp_fingerprint == other.pgp_fingerprint && self.image == other.image && self.twitter == other.twitter } @@ -365,7 +382,7 @@ impl> Clone for IdentityInfo { /// /// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a /// backwards compatible way through a specialized `Decode` impl. -#[derive(Clone, Encode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] +#[derive(Encode, Eq, MaxEncodedLen)] pub struct Registration< Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, MaxJudgments: Get, @@ -382,6 +399,58 @@ pub struct Registration< pub info: IdentityInfo, } +#[cfg(not(feature = "std"))] +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Debug for Registration { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "") + } +} + +#[cfg(feature = "std")] +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Debug for Registration { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Registration") + .field("judgements", &self.judgements) + .field("deposit", &self.deposit) + .field("info", &self.info) + .finish() + } +} + +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Clone for Registration { + fn clone(&self) -> Self { + Self { + judgements: self.judgements.clone(), + deposit: self.deposit.clone(), + info: self.info.clone(), + } + } +} + +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> PartialEq for Registration { + fn eq(&self, other: &Registration) -> bool { + self.judgements == other.judgements + && self.deposit == other.deposit + && self.info == other.info + } +} + impl < Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, MaxJudgments: Get, @@ -623,7 +692,6 @@ pub mod pallet { let (i, registrar_count) = >::try_mutate( |registrars| -> Result<(RegistrarIndex, usize), DispatchError> { - ensure!(registrars.len() < T::MaxRegistrars::get() as usize, Error::::TooManyRegistrars); registrars.try_push(Some(RegistrarInfo { account, fee: Zero::zero(), fields: Default::default() })) diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index c33a2b37be106..2bde868382cce 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -141,16 +141,16 @@ pub fn new_test_ext() -> sp_io::TestExternalities { fn ten() -> IdentityInfo { IdentityInfo { - display: Data::Raw(b"ten".to_vec()), - legal: Data::Raw(b"The Right Ordinal Ten, Esq.".to_vec()), + display: Data::Raw(b"ten".to_vec().try_into().unwrap()), + legal: Data::Raw(b"The Right Ordinal Ten, Esq.".to_vec().try_into().unwrap()), .. Default::default() } } fn twenty() -> IdentityInfo { IdentityInfo { - display: Data::Raw(b"twenty".to_vec()), - legal: Data::Raw(b"The Right Ordinal Twenty, Esq.".to_vec()), + display: Data::Raw(b"twenty".to_vec().try_into().unwrap()), + legal: Data::Raw(b"The Right Ordinal Twenty, Esq.".to_vec().try_into().unwrap()), .. Default::default() } } @@ -158,7 +158,7 @@ fn twenty() -> IdentityInfo { #[test] fn editing_subaccounts_should_work() { new_test_ext().execute_with(|| { - let data = |x| Data::Raw(vec![x; 1]); + let data = |x| Data::Raw(vec![x; 1].try_into().unwrap()); assert_noop!(Identity::add_sub(Origin::signed(10), 20, data(1)), Error::::NoIdentity); @@ -202,7 +202,7 @@ fn editing_subaccounts_should_work() { #[test] fn resolving_subaccount_ownership_works() { new_test_ext().execute_with(|| { - let data = |x| Data::Raw(vec![x; 1]); + let data = |x| Data::Raw(vec![x; 1].try_into().unwrap()); assert_ok!(Identity::set_identity(Origin::signed(10), ten())); assert_ok!(Identity::set_identity(Origin::signed(20), twenty())); @@ -227,11 +227,11 @@ fn resolving_subaccount_ownership_works() { #[test] fn trailing_zeros_decodes_into_default_data() { - let encoded = Data::Raw(b"Hello".to_vec()).encode(); + let encoded = Data::Raw(b"Hello".to_vec().try_into().unwrap()).encode(); assert!(<(Data, Data)>::decode(&mut &encoded[..]).is_err()); let input = &mut &encoded[..]; let (a, b) = <(Data, Data)>::decode(&mut AppendZerosInput::new(input)).unwrap(); - assert_eq!(a, Data::Raw(b"Hello".to_vec())); + assert_eq!(a, Data::Raw(b"Hello".to_vec().try_into().unwrap())); assert_eq!(b, Data::None); } @@ -268,13 +268,9 @@ fn registration_should_work() { assert_ok!(Identity::add_registrar(Origin::signed(1), 3)); assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10)); let mut three_fields = ten(); - three_fields.additional.push(Default::default()); - three_fields.additional.push(Default::default()); - three_fields.additional.push(Default::default()); - assert_noop!( - Identity::set_identity(Origin::signed(10), three_fields), - Error::::TooManyFields - ); + three_fields.additional.try_push(Default::default()).unwrap(); + three_fields.additional.try_push(Default::default()).unwrap(); + assert_eq!(three_fields.additional.try_push(Default::default()), Err(())); assert_ok!(Identity::set_identity(Origin::signed(10), ten())); assert_eq!(Identity::identity(10).unwrap().info, ten()); assert_eq!(Balances::free_balance(10), 90); @@ -339,31 +335,31 @@ fn killing_slashing_should_work() { #[test] fn setting_subaccounts_should_work() { new_test_ext().execute_with(|| { - let mut subs = vec![(20, Data::Raw(vec![40; 1]))]; + let mut subs = vec![(20, Data::Raw(vec![40; 1].try_into().unwrap()))]; assert_noop!(Identity::set_subs(Origin::signed(10), subs.clone()), Error::::NotFound); assert_ok!(Identity::set_identity(Origin::signed(10), ten())); assert_ok!(Identity::set_subs(Origin::signed(10), subs.clone())); assert_eq!(Balances::free_balance(10), 80); assert_eq!(Identity::subs_of(10), (10, vec![20])); - assert_eq!(Identity::super_of(20), Some((10, Data::Raw(vec![40; 1])))); + assert_eq!(Identity::super_of(20), Some((10, Data::Raw(vec![40; 1].try_into().unwrap())))); // push another item and re-set it. - subs.push((30, Data::Raw(vec![50; 1]))); + subs.push((30, Data::Raw(vec![50; 1].try_into().unwrap()))); assert_ok!(Identity::set_subs(Origin::signed(10), subs.clone())); assert_eq!(Balances::free_balance(10), 70); assert_eq!(Identity::subs_of(10), (20, vec![20, 30])); - assert_eq!(Identity::super_of(20), Some((10, Data::Raw(vec![40; 1])))); - assert_eq!(Identity::super_of(30), Some((10, Data::Raw(vec![50; 1])))); + assert_eq!(Identity::super_of(20), Some((10, Data::Raw(vec![40; 1].try_into().unwrap())))); + assert_eq!(Identity::super_of(30), Some((10, Data::Raw(vec![50; 1].try_into().unwrap())))); // switch out one of the items and re-set. - subs[0] = (40, Data::Raw(vec![60; 1])); + subs[0] = (40, Data::Raw(vec![60; 1].try_into().unwrap())); assert_ok!(Identity::set_subs(Origin::signed(10), subs.clone())); assert_eq!(Balances::free_balance(10), 70); // no change in the balance assert_eq!(Identity::subs_of(10), (20, vec![40, 30])); assert_eq!(Identity::super_of(20), None); - assert_eq!(Identity::super_of(30), Some((10, Data::Raw(vec![50; 1])))); - assert_eq!(Identity::super_of(40), Some((10, Data::Raw(vec![60; 1])))); + assert_eq!(Identity::super_of(30), Some((10, Data::Raw(vec![50; 1].try_into().unwrap())))); + assert_eq!(Identity::super_of(40), Some((10, Data::Raw(vec![60; 1].try_into().unwrap())))); // clear assert_ok!(Identity::set_subs(Origin::signed(10), vec![])); @@ -372,7 +368,7 @@ fn setting_subaccounts_should_work() { assert_eq!(Identity::super_of(30), None); assert_eq!(Identity::super_of(40), None); - subs.push((20, Data::Raw(vec![40; 1]))); + subs.push((20, Data::Raw(vec![40; 1].try_into().unwrap()))); assert_noop!(Identity::set_subs(Origin::signed(10), subs.clone()), Error::::TooManySubAccounts); }); } @@ -381,7 +377,7 @@ fn setting_subaccounts_should_work() { fn clearing_account_should_remove_subaccounts_and_refund() { new_test_ext().execute_with(|| { assert_ok!(Identity::set_identity(Origin::signed(10), ten())); - assert_ok!(Identity::set_subs(Origin::signed(10), vec![(20, Data::Raw(vec![40; 1]))])); + assert_ok!(Identity::set_subs(Origin::signed(10), vec![(20, Data::Raw(vec![40; 1].try_into().unwrap()))])); assert_ok!(Identity::clear_identity(Origin::signed(10))); assert_eq!(Balances::free_balance(10), 100); assert!(Identity::super_of(20).is_none()); @@ -392,7 +388,7 @@ fn clearing_account_should_remove_subaccounts_and_refund() { fn killing_account_should_remove_subaccounts_and_not_refund() { new_test_ext().execute_with(|| { assert_ok!(Identity::set_identity(Origin::signed(10), ten())); - assert_ok!(Identity::set_subs(Origin::signed(10), vec![(20, Data::Raw(vec![40; 1]))])); + assert_ok!(Identity::set_subs(Origin::signed(10), vec![(20, Data::Raw(vec![40; 1].try_into().unwrap()))])); assert_ok!(Identity::kill_identity(Origin::signed(2), 10)); assert_eq!(Balances::free_balance(10), 80); assert!(Identity::super_of(20).is_none()); @@ -453,9 +449,11 @@ fn field_deposit_should_work() { assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10)); assert_ok!(Identity::set_identity(Origin::signed(10), IdentityInfo { additional: vec![ - (Data::Raw(b"number".to_vec()), Data::Raw(10u32.encode())), - (Data::Raw(b"text".to_vec()), Data::Raw(b"10".to_vec())), - ], .. Default::default() + (Data::Raw(b"number".to_vec().try_into().unwrap()), Data::Raw(10u32.encode().try_into().unwrap())), + (Data::Raw(b"text".to_vec().try_into().unwrap()), Data::Raw(b"10".to_vec().try_into().unwrap())), + ] + .try_into() + .unwrap(), .. Default::default() })); assert_eq!(Balances::free_balance(10), 70); }); From aee3907b831f1d487762fdf6a9ecaa0a099bb415 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 01:54:32 -0700 Subject: [PATCH 03/18] Move identity pallet custom types into its own module --- frame/identity/src/lib.rs | 409 +--------------------------------- frame/identity/src/tests.rs | 1 + frame/identity/src/types.rs | 427 ++++++++++++++++++++++++++++++++++++ 3 files changed, 432 insertions(+), 405 deletions(-) create mode 100644 frame/identity/src/types.rs diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index a4bd3f05831e1..682c5ea48052e 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -74,423 +74,22 @@ #[cfg(test)] mod tests; +mod types; mod benchmarking; pub mod weights; use sp_std::prelude::*; -use sp_std::{convert::TryInto, fmt::{self, Debug}, ops::Add, iter::once}; -use enumflags2::BitFlags; -use codec::{Encode, Decode}; -use sp_runtime::RuntimeDebug; +use sp_std::convert::TryInto; use sp_runtime::traits::{StaticLookup, Zero, AppendZerosInput, Saturating}; -use frame_support::{ - parameter_types, - traits::{BalanceStatus, Currency, Get, MaxEncodedLen, OnUnbalanced, ReservableCurrency}, - BoundedVec, -}; +use frame_support::traits::{BalanceStatus, Currency, OnUnbalanced, ReservableCurrency}; pub use weights::WeightInfo; pub use pallet::*; +use types::*; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; -parameter_types! { - pub const MaxDataSize: u32 = 32; -} - -/// Either underlying data blob if it is at most 32 bytes, or a hash of it. If the data is greater -/// than 32-bytes then it will be truncated when encoding. -/// -/// Can also be `None`. -#[derive(Clone, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] -pub enum Data { - /// No data here. - None, - /// The data is stored directly. - Raw(BoundedVec), - /// Only the Blake2 hash of the data is stored. The preimage of the hash may be retrieved - /// through some hash-lookup service. - BlakeTwo256([u8; 32]), - /// Only the SHA2-256 hash of the data is stored. The preimage of the hash may be retrieved - /// through some hash-lookup service. - Sha256([u8; 32]), - /// Only the Keccak-256 hash of the data is stored. The preimage of the hash may be retrieved - /// through some hash-lookup service. - Keccak256([u8; 32]), - /// Only the SHA3-256 hash of the data is stored. The preimage of the hash may be retrieved - /// through some hash-lookup service. - ShaThree256([u8; 32]), -} - -impl Decode for Data { - fn decode(input: &mut I) -> sp_std::result::Result { - let b = input.read_byte()?; - Ok(match b { - 0 => Data::None, - n @ 1 ..= 33 => { - let mut r: BoundedVec<_, _> = vec![0u8; n as usize - 1] - .try_into() - .expect("bound checked in match arm condition; qed"); - input.read(&mut r[..])?; - Data::Raw(r) - } - 34 => Data::BlakeTwo256(<[u8; 32]>::decode(input)?), - 35 => Data::Sha256(<[u8; 32]>::decode(input)?), - 36 => Data::Keccak256(<[u8; 32]>::decode(input)?), - 37 => Data::ShaThree256(<[u8; 32]>::decode(input)?), - _ => return Err(codec::Error::from("invalid leading byte")), - }) - } -} - -impl Encode for Data { - fn encode(&self) -> Vec { - match self { - Data::None => vec![0u8; 1], - Data::Raw(ref x) => { - let l = x.len().min(32); - let mut r = vec![l as u8 + 1; l + 1]; - &mut r[1..].copy_from_slice(&x[..l as usize]); - r - } - Data::BlakeTwo256(ref h) => once(34u8).chain(h.iter().cloned()).collect(), - Data::Sha256(ref h) => once(35u8).chain(h.iter().cloned()).collect(), - Data::Keccak256(ref h) => once(36u8).chain(h.iter().cloned()).collect(), - Data::ShaThree256(ref h) => once(37u8).chain(h.iter().cloned()).collect(), - } - } -} -impl codec::EncodeLike for Data {} - -impl Default for Data { - fn default() -> Self { - Self::None - } -} - -/// An identifier for a single name registrar/identity verification service. -pub type RegistrarIndex = u32; - -/// An attestation of a registrar over how accurate some `IdentityInfo` is in describing an account. -/// -/// NOTE: Registrars may pay little attention to some fields. Registrars may want to make clear -/// which fields their attestation is relevant for by off-chain means. -#[derive(Copy, Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] -pub enum Judgement< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq -> { - /// The default value; no opinion is held. - Unknown, - /// No judgement is yet in place, but a deposit is reserved as payment for providing one. - FeePaid(Balance), - /// The data appears to be reasonably acceptable in terms of its accuracy, however no in depth - /// checks (such as in-person meetings or formal KYC) have been conducted. - Reasonable, - /// The target is known directly by the registrar and the registrar can fully attest to the - /// the data's accuracy. - KnownGood, - /// The data was once good but is currently out of date. There is no malicious intent in the - /// inaccuracy. This judgement can be removed through updating the data. - OutOfDate, - /// The data is imprecise or of sufficiently low-quality to be problematic. It is not - /// indicative of malicious intent. This judgement can be removed through updating the data. - LowQuality, - /// The data is erroneous. This may be indicative of malicious intent. This cannot be removed - /// except by the registrar. - Erroneous, -} - -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq -> Judgement { - /// Returns `true` if this judgement is indicative of a deposit being currently held. This means - /// it should not be cleared or replaced except by an operation which utilizes the deposit. - fn has_deposit(&self) -> bool { - match self { - Judgement::FeePaid(_) => true, - _ => false, - } - } - - /// Returns `true` if this judgement is one that should not be generally be replaced outside - /// of specialized handlers. Examples include "malicious" judgements and deposit-holding - /// judgements. - fn is_sticky(&self) -> bool { - match self { - Judgement::FeePaid(_) | Judgement::Erroneous => true, - _ => false, - } - } -} - -/// The fields that we use to identify the owner of an account with. Each corresponds to a field -/// in the `IdentityInfo` struct. -#[repr(u64)] -#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, BitFlags, RuntimeDebug)] -pub enum IdentityField { - Display = 0b0000000000000000000000000000000000000000000000000000000000000001, - Legal = 0b0000000000000000000000000000000000000000000000000000000000000010, - Web = 0b0000000000000000000000000000000000000000000000000000000000000100, - Riot = 0b0000000000000000000000000000000000000000000000000000000000001000, - Email = 0b0000000000000000000000000000000000000000000000000000000000010000, - PgpFingerprint = 0b0000000000000000000000000000000000000000000000000000000000100000, - Image = 0b0000000000000000000000000000000000000000000000000000000001000000, - Twitter = 0b0000000000000000000000000000000000000000000000000000000010000000, -} - -/// Wrapper type for `BitFlags` that implements `Codec`. -#[derive(Clone, Copy, PartialEq, Default, RuntimeDebug)] -pub struct IdentityFields(BitFlags); - -impl Eq for IdentityFields {} -impl Encode for IdentityFields { - fn using_encoded R>(&self, f: F) -> R { - self.0.bits().using_encoded(f) - } -} -impl Decode for IdentityFields { - fn decode(input: &mut I) -> sp_std::result::Result { - let field = u64::decode(input)?; - Ok(Self(>::from_bits(field as u64).map_err(|_| "invalid value")?)) - } -} - -/// Information concerning the identity of the controller of an account. -/// -/// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra -/// fields in a backwards compatible way through a specialized `Decode` impl. -#[derive(Encode, Decode, Eq, MaxEncodedLen)] -pub struct IdentityInfo> { - /// Additional fields of the identity that are not catered for with the struct's explicit - /// fields. - pub additional: BoundedVec<(Data, Data), FieldLimit>, - - /// A reasonable display name for the controller of the account. This should be whatever it is - /// that it is typically known as and should not be confusable with other entities, given - /// reasonable context. - /// - /// Stored as UTF-8. - pub display: Data, - - /// The full legal name in the local jurisdiction of the entity. This might be a bit - /// long-winded. - /// - /// Stored as UTF-8. - pub legal: Data, - - /// A representative website held by the controller of the account. - /// - /// NOTE: `https://` is automatically prepended. - /// - /// Stored as UTF-8. - pub web: Data, - - /// The Riot/Matrix handle held by the controller of the account. - /// - /// Stored as UTF-8. - pub riot: Data, - - /// The email address of the controller of the account. - /// - /// Stored as UTF-8. - pub email: Data, - - /// The PGP/GPG public key of the controller of the account. - pub pgp_fingerprint: Option<[u8; 20]>, - - /// A graphic image representing the controller of the account. Should be a company, - /// organization or project logo or a headshot in the case of a human. - pub image: Data, - - /// The Twitter identity. The leading `@` character may be elided. - pub twitter: Data, -} - -#[cfg(test)] -impl> Default for IdentityInfo { - fn default() -> Self { - Self { - additional: Default::default(), - display: Default::default(), - legal: Default::default(), - web: Default::default(), - riot: Default::default(), - email: Default::default(), - pgp_fingerprint: Default::default(), - image: Default::default(), - twitter: Default::default(), - } - } -} - -#[cfg(not(feature = "std"))] -impl> Debug for IdentityInfo { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "") - } -} - -#[cfg(feature = "std")] -impl> Debug for IdentityInfo { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("IdentityInfo") - .field("additional", &self.additional) - .field("display", &self.display) - .field("legal", &self.legal) - .field("web", &self.web) - .field("riot", &self.riot) - .field("email", &self.email) - .field("pgp_fingerprint", &self.pgp_fingerprint) - .field("image", &self.image) - .field("twitter", &self.twitter) - .finish() - } -} - -impl> PartialEq for IdentityInfo { - fn eq(&self, other: &IdentityInfo) -> bool { - self.additional == other.additional - && self.display == other.display - && self.legal == other.legal - && self.web == other.web - && self.riot == other.riot - && self.email == other.email - && self.pgp_fingerprint == other.pgp_fingerprint - && self.image == other.image - && self.twitter == other.twitter - } -} - -impl> Clone for IdentityInfo { - fn clone(&self) -> Self { - Self { - additional: self.additional.clone(), - display: self.display.clone(), - legal: self.legal.clone(), - web: self.web.clone(), - riot: self.riot.clone(), - email: self.email.clone(), - pgp_fingerprint: self.pgp_fingerprint.clone(), - image: self.image.clone(), - twitter: self.twitter.clone(), - } - } -} - -/// Information concerning the identity of the controller of an account. -/// -/// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a -/// backwards compatible way through a specialized `Decode` impl. -#[derive(Encode, Eq, MaxEncodedLen)] -pub struct Registration< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> { - /// Judgements from the registrars on this identity. Stored ordered by `RegistrarIndex`. There - /// may be only a single judgement from each registrar. - pub judgements: BoundedVec<(RegistrarIndex, Judgement), MaxJudgments>, - - /// Amount held on deposit for this information. - pub deposit: Balance, - - /// Information on the identity. - pub info: IdentityInfo, -} - -#[cfg(not(feature = "std"))] -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Debug for Registration { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "") - } -} - -#[cfg(feature = "std")] -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Debug for Registration { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Registration") - .field("judgements", &self.judgements) - .field("deposit", &self.deposit) - .field("info", &self.info) - .finish() - } -} - -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Clone for Registration { - fn clone(&self) -> Self { - Self { - judgements: self.judgements.clone(), - deposit: self.deposit.clone(), - info: self.info.clone(), - } - } -} - -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> PartialEq for Registration { - fn eq(&self, other: &Registration) -> bool { - self.judgements == other.judgements - && self.deposit == other.deposit - && self.info == other.info - } -} - -impl < - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Registration { - fn total_deposit(&self) -> Balance { - self.deposit + self.judgements.iter() - .map(|(_, ref j)| if let Judgement::FeePaid(fee) = j { *fee } else { Zero::zero() }) - .fold(Zero::zero(), |a, i| a + i) - } -} - -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Decode for Registration { - fn decode(input: &mut I) -> sp_std::result::Result { - let (judgements, deposit, info) = Decode::decode(&mut AppendZerosInput::new(input))?; - Ok(Self { judgements, deposit, info }) - } -} - -/// Information concerning a registrar. -#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug)] -pub struct RegistrarInfo< - Balance: Encode + Decode + Clone + Debug + Eq + PartialEq, - AccountId: Encode + Decode + Clone + Debug + Eq + PartialEq -> { - /// The account of the registrar. - pub account: AccountId, - - /// Amount required to be given to the registrar for them to provide judgement. - pub fee: Balance, - - /// Relevant fields for this registrar. Registrar judgements are limited to attestations on - /// these fields. - pub fields: IdentityFields, -} - #[frame_support::pallet] pub mod pallet { use frame_support::pallet_prelude::*; diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index 2bde868382cce..d6b755df8d351 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -20,6 +20,7 @@ use super::*; use crate as pallet_identity; +use codec::{Encode, Decode}; use sp_runtime::traits::BadOrigin; use frame_support::{assert_ok, assert_noop, parameter_types, ord_parameter_types}; use sp_core::H256; diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs new file mode 100644 index 0000000000000..4a53e7cbe4deb --- /dev/null +++ b/frame/identity/src/types.rs @@ -0,0 +1,427 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 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. + +use codec::{Encode, Decode}; +use enumflags2::BitFlags; +use frame_support::{ + parameter_types, + traits::{Get, MaxEncodedLen}, + BoundedVec, +}; +use sp_std::prelude::*; +use sp_std::{fmt::{self, Debug}, iter::once, ops::Add}; +use sp_runtime::{ + traits::Zero, + RuntimeDebug, +}; +use super::*; + +parameter_types! { + pub const MaxDataSize: u32 = 32; +} + +/// Either underlying data blob if it is at most 32 bytes, or a hash of it. If the data is greater +/// than 32-bytes then it will be truncated when encoding. +/// +/// Can also be `None`. +#[derive(Clone, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] +pub enum Data { + /// No data here. + None, + /// The data is stored directly. + Raw(BoundedVec), + /// Only the Blake2 hash of the data is stored. The preimage of the hash may be retrieved + /// through some hash-lookup service. + BlakeTwo256([u8; 32]), + /// Only the SHA2-256 hash of the data is stored. The preimage of the hash may be retrieved + /// through some hash-lookup service. + Sha256([u8; 32]), + /// Only the Keccak-256 hash of the data is stored. The preimage of the hash may be retrieved + /// through some hash-lookup service. + Keccak256([u8; 32]), + /// Only the SHA3-256 hash of the data is stored. The preimage of the hash may be retrieved + /// through some hash-lookup service. + ShaThree256([u8; 32]), +} + +impl Decode for Data { + fn decode(input: &mut I) -> sp_std::result::Result { + let b = input.read_byte()?; + Ok(match b { + 0 => Data::None, + n @ 1 ..= 33 => { + let mut r: BoundedVec<_, _> = vec![0u8; n as usize - 1] + .try_into() + .expect("bound checked in match arm condition; qed"); + input.read(&mut r[..])?; + Data::Raw(r) + } + 34 => Data::BlakeTwo256(<[u8; 32]>::decode(input)?), + 35 => Data::Sha256(<[u8; 32]>::decode(input)?), + 36 => Data::Keccak256(<[u8; 32]>::decode(input)?), + 37 => Data::ShaThree256(<[u8; 32]>::decode(input)?), + _ => return Err(codec::Error::from("invalid leading byte")), + }) + } +} + +impl Encode for Data { + fn encode(&self) -> Vec { + match self { + Data::None => vec![0u8; 1], + Data::Raw(ref x) => { + let l = x.len().min(32); + let mut r = vec![l as u8 + 1; l + 1]; + &mut r[1..].copy_from_slice(&x[..l as usize]); + r + } + Data::BlakeTwo256(ref h) => once(34u8).chain(h.iter().cloned()).collect(), + Data::Sha256(ref h) => once(35u8).chain(h.iter().cloned()).collect(), + Data::Keccak256(ref h) => once(36u8).chain(h.iter().cloned()).collect(), + Data::ShaThree256(ref h) => once(37u8).chain(h.iter().cloned()).collect(), + } + } +} +impl codec::EncodeLike for Data {} + +impl Default for Data { + fn default() -> Self { + Self::None + } +} + +/// An identifier for a single name registrar/identity verification service. +pub type RegistrarIndex = u32; + +/// An attestation of a registrar over how accurate some `IdentityInfo` is in describing an account. +/// +/// NOTE: Registrars may pay little attention to some fields. Registrars may want to make clear +/// which fields their attestation is relevant for by off-chain means. +#[derive(Copy, Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] +pub enum Judgement< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq +> { + /// The default value; no opinion is held. + Unknown, + /// No judgement is yet in place, but a deposit is reserved as payment for providing one. + FeePaid(Balance), + /// The data appears to be reasonably acceptable in terms of its accuracy, however no in depth + /// checks (such as in-person meetings or formal KYC) have been conducted. + Reasonable, + /// The target is known directly by the registrar and the registrar can fully attest to the + /// the data's accuracy. + KnownGood, + /// The data was once good but is currently out of date. There is no malicious intent in the + /// inaccuracy. This judgement can be removed through updating the data. + OutOfDate, + /// The data is imprecise or of sufficiently low-quality to be problematic. It is not + /// indicative of malicious intent. This judgement can be removed through updating the data. + LowQuality, + /// The data is erroneous. This may be indicative of malicious intent. This cannot be removed + /// except by the registrar. + Erroneous, +} + +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq +> Judgement { + /// Returns `true` if this judgement is indicative of a deposit being currently held. This means + /// it should not be cleared or replaced except by an operation which utilizes the deposit. + pub(crate) fn has_deposit(&self) -> bool { + match self { + Judgement::FeePaid(_) => true, + _ => false, + } + } + + /// Returns `true` if this judgement is one that should not be generally be replaced outside + /// of specialized handlers. Examples include "malicious" judgements and deposit-holding + /// judgements. + pub(crate) fn is_sticky(&self) -> bool { + match self { + Judgement::FeePaid(_) | Judgement::Erroneous => true, + _ => false, + } + } +} + +/// The fields that we use to identify the owner of an account with. Each corresponds to a field +/// in the `IdentityInfo` struct. +#[repr(u64)] +#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, BitFlags, RuntimeDebug)] +pub enum IdentityField { + Display = 0b0000000000000000000000000000000000000000000000000000000000000001, + Legal = 0b0000000000000000000000000000000000000000000000000000000000000010, + Web = 0b0000000000000000000000000000000000000000000000000000000000000100, + Riot = 0b0000000000000000000000000000000000000000000000000000000000001000, + Email = 0b0000000000000000000000000000000000000000000000000000000000010000, + PgpFingerprint = 0b0000000000000000000000000000000000000000000000000000000000100000, + Image = 0b0000000000000000000000000000000000000000000000000000000001000000, + Twitter = 0b0000000000000000000000000000000000000000000000000000000010000000, +} + +/// Wrapper type for `BitFlags` that implements `Codec`. +#[derive(Clone, Copy, PartialEq, Default, RuntimeDebug)] +pub struct IdentityFields(pub(crate) BitFlags); + +impl Eq for IdentityFields {} +impl Encode for IdentityFields { + fn using_encoded R>(&self, f: F) -> R { + self.0.bits().using_encoded(f) + } +} +impl Decode for IdentityFields { + fn decode(input: &mut I) -> sp_std::result::Result { + let field = u64::decode(input)?; + Ok(Self(>::from_bits(field as u64).map_err(|_| "invalid value")?)) + } +} + +/// Information concerning the identity of the controller of an account. +/// +/// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra +/// fields in a backwards compatible way through a specialized `Decode` impl. +#[derive(Encode, Decode, Eq, MaxEncodedLen)] +pub struct IdentityInfo> { + /// Additional fields of the identity that are not catered for with the struct's explicit + /// fields. + pub additional: BoundedVec<(Data, Data), FieldLimit>, + + /// A reasonable display name for the controller of the account. This should be whatever it is + /// that it is typically known as and should not be confusable with other entities, given + /// reasonable context. + /// + /// Stored as UTF-8. + pub display: Data, + + /// The full legal name in the local jurisdiction of the entity. This might be a bit + /// long-winded. + /// + /// Stored as UTF-8. + pub legal: Data, + + /// A representative website held by the controller of the account. + /// + /// NOTE: `https://` is automatically prepended. + /// + /// Stored as UTF-8. + pub web: Data, + + /// The Riot/Matrix handle held by the controller of the account. + /// + /// Stored as UTF-8. + pub riot: Data, + + /// The email address of the controller of the account. + /// + /// Stored as UTF-8. + pub email: Data, + + /// The PGP/GPG public key of the controller of the account. + pub pgp_fingerprint: Option<[u8; 20]>, + + /// A graphic image representing the controller of the account. Should be a company, + /// organization or project logo or a headshot in the case of a human. + pub image: Data, + + /// The Twitter identity. The leading `@` character may be elided. + pub twitter: Data, +} + +#[cfg(test)] +impl> Default for IdentityInfo { + fn default() -> Self { + Self { + additional: Default::default(), + display: Default::default(), + legal: Default::default(), + web: Default::default(), + riot: Default::default(), + email: Default::default(), + pgp_fingerprint: Default::default(), + image: Default::default(), + twitter: Default::default(), + } + } +} + +#[cfg(not(feature = "std"))] +impl> Debug for IdentityInfo { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "") + } +} + +#[cfg(feature = "std")] +impl> Debug for IdentityInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("IdentityInfo") + .field("additional", &self.additional) + .field("display", &self.display) + .field("legal", &self.legal) + .field("web", &self.web) + .field("riot", &self.riot) + .field("email", &self.email) + .field("pgp_fingerprint", &self.pgp_fingerprint) + .field("image", &self.image) + .field("twitter", &self.twitter) + .finish() + } +} + +impl> PartialEq for IdentityInfo { + fn eq(&self, other: &IdentityInfo) -> bool { + self.additional == other.additional + && self.display == other.display + && self.legal == other.legal + && self.web == other.web + && self.riot == other.riot + && self.email == other.email + && self.pgp_fingerprint == other.pgp_fingerprint + && self.image == other.image + && self.twitter == other.twitter + } +} + +impl> Clone for IdentityInfo { + fn clone(&self) -> Self { + Self { + additional: self.additional.clone(), + display: self.display.clone(), + legal: self.legal.clone(), + web: self.web.clone(), + riot: self.riot.clone(), + email: self.email.clone(), + pgp_fingerprint: self.pgp_fingerprint.clone(), + image: self.image.clone(), + twitter: self.twitter.clone(), + } + } +} + +/// Information concerning the identity of the controller of an account. +/// +/// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a +/// backwards compatible way through a specialized `Decode` impl. +#[derive(Encode, Eq, MaxEncodedLen)] +pub struct Registration< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> { + /// Judgements from the registrars on this identity. Stored ordered by `RegistrarIndex`. There + /// may be only a single judgement from each registrar. + pub judgements: BoundedVec<(RegistrarIndex, Judgement), MaxJudgments>, + + /// Amount held on deposit for this information. + pub deposit: Balance, + + /// Information on the identity. + pub info: IdentityInfo, +} + +#[cfg(not(feature = "std"))] +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Debug for Registration { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "") + } +} + +#[cfg(feature = "std")] +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Debug for Registration { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Registration") + .field("judgements", &self.judgements) + .field("deposit", &self.deposit) + .field("info", &self.info) + .finish() + } +} + +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Clone for Registration { + fn clone(&self) -> Self { + Self { + judgements: self.judgements.clone(), + deposit: self.deposit.clone(), + info: self.info.clone(), + } + } +} + +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> PartialEq for Registration { + fn eq(&self, other: &Registration) -> bool { + self.judgements == other.judgements + && self.deposit == other.deposit + && self.info == other.info + } +} + +impl < + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Registration { + pub(crate) fn total_deposit(&self) -> Balance { + self.deposit + self.judgements.iter() + .map(|(_, ref j)| if let Judgement::FeePaid(fee) = j { *fee } else { Zero::zero() }) + .fold(Zero::zero(), |a, i| a + i) + } +} + +impl< + Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgments: Get, + MaxAdditionalFields: Get, +> Decode for Registration { + fn decode(input: &mut I) -> sp_std::result::Result { + let (judgements, deposit, info) = Decode::decode(&mut AppendZerosInput::new(input))?; + Ok(Self { judgements, deposit, info }) + } +} + +/// Information concerning a registrar. +#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug)] +pub struct RegistrarInfo< + Balance: Encode + Decode + Clone + Debug + Eq + PartialEq, + AccountId: Encode + Decode + Clone + Debug + Eq + PartialEq +> { + /// The account of the registrar. + pub account: AccountId, + + /// Amount required to be given to the registrar for them to provide judgement. + pub fee: Balance, + + /// Relevant fields for this registrar. Registrar judgements are limited to attestations on + /// these fields. + pub fields: IdentityFields, +} From 0ec33a235bc3f60453bff590881ae325f3766951 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 02:03:26 -0700 Subject: [PATCH 04/18] Make use of NoBound family traits --- frame/identity/src/types.rs | 114 ++---------------------------------- 1 file changed, 4 insertions(+), 110 deletions(-) diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs index 4a53e7cbe4deb..44faa32ff9941 100644 --- a/frame/identity/src/types.rs +++ b/frame/identity/src/types.rs @@ -20,10 +20,10 @@ use enumflags2::BitFlags; use frame_support::{ parameter_types, traits::{Get, MaxEncodedLen}, - BoundedVec, + BoundedVec, CloneNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; use sp_std::prelude::*; -use sp_std::{fmt::{self, Debug}, iter::once, ops::Add}; +use sp_std::{fmt::Debug, iter::once, ops::Add}; use sp_runtime::{ traits::Zero, RuntimeDebug, @@ -195,7 +195,7 @@ impl Decode for IdentityFields { /// /// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra /// fields in a backwards compatible way through a specialized `Decode` impl. -#[derive(Encode, Decode, Eq, MaxEncodedLen)] +#[derive(CloneNoBound, Encode, Decode, Eq, MaxEncodedLen, PartialEqNoBound, RuntimeDebugNoBound)] pub struct IdentityInfo> { /// Additional fields of the identity that are not catered for with the struct's explicit /// fields. @@ -259,65 +259,11 @@ impl> Default for IdentityInfo { } } -#[cfg(not(feature = "std"))] -impl> Debug for IdentityInfo { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "") - } -} - -#[cfg(feature = "std")] -impl> Debug for IdentityInfo { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("IdentityInfo") - .field("additional", &self.additional) - .field("display", &self.display) - .field("legal", &self.legal) - .field("web", &self.web) - .field("riot", &self.riot) - .field("email", &self.email) - .field("pgp_fingerprint", &self.pgp_fingerprint) - .field("image", &self.image) - .field("twitter", &self.twitter) - .finish() - } -} - -impl> PartialEq for IdentityInfo { - fn eq(&self, other: &IdentityInfo) -> bool { - self.additional == other.additional - && self.display == other.display - && self.legal == other.legal - && self.web == other.web - && self.riot == other.riot - && self.email == other.email - && self.pgp_fingerprint == other.pgp_fingerprint - && self.image == other.image - && self.twitter == other.twitter - } -} - -impl> Clone for IdentityInfo { - fn clone(&self) -> Self { - Self { - additional: self.additional.clone(), - display: self.display.clone(), - legal: self.legal.clone(), - web: self.web.clone(), - riot: self.riot.clone(), - email: self.email.clone(), - pgp_fingerprint: self.pgp_fingerprint.clone(), - image: self.image.clone(), - twitter: self.twitter.clone(), - } - } -} - /// Information concerning the identity of the controller of an account. /// /// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a /// backwards compatible way through a specialized `Decode` impl. -#[derive(Encode, Eq, MaxEncodedLen)] +#[derive(CloneNoBound, Encode, Eq, MaxEncodedLen, PartialEqNoBound, RuntimeDebugNoBound)] pub struct Registration< Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, MaxJudgments: Get, @@ -334,58 +280,6 @@ pub struct Registration< pub info: IdentityInfo, } -#[cfg(not(feature = "std"))] -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Debug for Registration { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "") - } -} - -#[cfg(feature = "std")] -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Debug for Registration { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Registration") - .field("judgements", &self.judgements) - .field("deposit", &self.deposit) - .field("info", &self.info) - .finish() - } -} - -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> Clone for Registration { - fn clone(&self) -> Self { - Self { - judgements: self.judgements.clone(), - deposit: self.deposit.clone(), - info: self.info.clone(), - } - } -} - -impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, - MaxAdditionalFields: Get, -> PartialEq for Registration { - fn eq(&self, other: &Registration) -> bool { - self.judgements == other.judgements - && self.deposit == other.deposit - && self.info == other.info - } -} - impl < Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, MaxJudgments: Get, From 86504cc0031ec554c8dc87676ad6cc1d8373d89b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 02:32:47 -0700 Subject: [PATCH 05/18] Fix identity pallet benchmarks --- frame/identity/src/benchmarking.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index 0cd2d50529dd1..bf8678412d940 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -84,11 +84,11 @@ fn add_sub_accounts(who: &T::AccountId, s: u32) -> Result(num_fields: u32) -> IdentityInfo { +fn create_identity_info(num_fields: u32) -> IdentityInfo { let data = Data::Raw(vec![0; 32]); let info = IdentityInfo { - additional: vec![(data.clone(), data.clone()); num_fields as usize], + additional: vec![(data.clone(), data.clone()); num_fields as usize].try_into().unwrap(), display: data.clone(), legal: data.clone(), web: data.clone(), From f5d790eef87e31f3be66569b99907139090f5b46 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 02:55:43 -0700 Subject: [PATCH 06/18] Enumerate type imports --- frame/identity/src/lib.rs | 2 +- frame/identity/src/tests.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index 682c5ea48052e..98fc220159874 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -85,7 +85,7 @@ use frame_support::traits::{BalanceStatus, Currency, OnUnbalanced, ReservableCur pub use weights::WeightInfo; pub use pallet::*; -use types::*; +use types::{Data, IdentityFields, IdentityInfo, Judgement, RegistrarIndex, RegistrarInfo, Registration}; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index d6b755df8d351..60166571822c8 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -28,6 +28,7 @@ use frame_system::{EnsureSignedBy, EnsureOneOf, EnsureRoot}; use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, }; +use types::IdentityField; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; From d9fb577846a70c6de4bee8d5424a70dc14052b1c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 02:58:05 -0700 Subject: [PATCH 07/18] Properly convert to BoundedVec in benchmarks --- frame/identity/src/benchmarking.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index bf8678412d940..4fb76fcb4138c 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -56,7 +56,7 @@ fn add_registrars(r: u32) -> Result<(), &'static str> { fn create_sub_accounts(who: &T::AccountId, s: u32) -> Result, &'static str> { let mut subs = Vec::new(); let who_origin = RawOrigin::Signed(who.clone()); - let data = Data::Raw(vec![0; 32]); + let data = Data::Raw(vec![0; 32].try_into().unwrap()); for i in 0..s { let sub_account = account("sub", i, SEED); @@ -85,7 +85,7 @@ fn add_sub_accounts(who: &T::AccountId, s: u32) -> Result(num_fields: u32) -> IdentityInfo { - let data = Data::Raw(vec![0; 32]); + let data = Data::Raw(vec![0; 32].try_into().unwrap()); let info = IdentityInfo { additional: vec![(data.clone(), data.clone()); num_fields as usize].try_into().unwrap(), @@ -353,7 +353,7 @@ benchmarks! { let caller: T::AccountId = whitelisted_caller(); let _ = add_sub_accounts::(&caller, s)?; let sub = account("new_sub", 0, SEED); - let data = Data::Raw(vec![0; 32]); + let data = Data::Raw(vec![0; 32].try_into().unwrap()); ensure!(SubsOf::::get(&caller).1.len() as u32 == s, "Subs not set."); }: _(RawOrigin::Signed(caller.clone()), T::Lookup::unlookup(sub), data) verify { @@ -365,7 +365,7 @@ benchmarks! { let caller: T::AccountId = whitelisted_caller(); let (sub, _) = add_sub_accounts::(&caller, s)?.remove(0); - let data = Data::Raw(vec![1; 32]); + let data = Data::Raw(vec![1; 32].try_into().unwrap()); ensure!(SuperOf::::get(&sub).unwrap().1 != data, "data already set"); }: _(RawOrigin::Signed(caller), T::Lookup::unlookup(sub.clone()), data.clone()) verify { @@ -390,7 +390,7 @@ benchmarks! { let sup = account("super", 0, SEED); let _ = add_sub_accounts::(&sup, s)?; let sup_origin = RawOrigin::Signed(sup).into(); - Identity::::add_sub(sup_origin, T::Lookup::unlookup(caller.clone()), Data::Raw(vec![0; 32]))?; + Identity::::add_sub(sup_origin, T::Lookup::unlookup(caller.clone()), Data::Raw(vec![0; 32].try_into().unwrap()))?; ensure!(SuperOf::::contains_key(&caller), "Sub doesn't exists"); }: _(RawOrigin::Signed(caller.clone())) verify { From 80d94b77895cb5258f3c2d076209e0a0fca223a8 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 03:03:11 -0700 Subject: [PATCH 08/18] Re-export types --- frame/identity/src/lib.rs | 5 ++++- frame/identity/src/tests.rs | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index 98fc220159874..badaffdba8953 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -85,7 +85,10 @@ use frame_support::traits::{BalanceStatus, Currency, OnUnbalanced, ReservableCur pub use weights::WeightInfo; pub use pallet::*; -use types::{Data, IdentityFields, IdentityInfo, Judgement, RegistrarIndex, RegistrarInfo, Registration}; +pub use types::{ + Data, IdentityField, IdentityFields, IdentityInfo, Judgement, RegistrarIndex, + RegistrarInfo, Registration, +}; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index 60166571822c8..d6b755df8d351 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -28,7 +28,6 @@ use frame_system::{EnsureSignedBy, EnsureOneOf, EnsureRoot}; use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, }; -use types::IdentityField; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; From ddf65eff9b2de87c839912c309f72646f93cef22 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 15:12:27 -0700 Subject: [PATCH 09/18] Use BoundedVec when storing sub identities --- frame/identity/src/lib.rs | 17 +++++++++++------ frame/identity/src/tests.rs | 10 +++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index badaffdba8953..8f3bf97b5a2ef 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -189,7 +189,7 @@ pub mod pallet { _, Twox64Concat, T::AccountId, - (BalanceOf, Vec), + (BalanceOf, BoundedVec), ValueQuery, >; @@ -418,10 +418,15 @@ pub mod pallet { for s in old_ids.iter() { >::remove(s); } - let ids = subs.into_iter().map(|(id, name)| { - >::insert(&id, (sender.clone(), name)); - id - }).collect::>(); + let ids: BoundedVec<_, _> = subs + .into_iter() + .map(|(id, name)| { + >::insert(&id, (sender.clone(), name)); + id + }) + .collect::>() + .try_into() + .map_err(|_| Error::::TooManySubAccounts)?; let new_subs = ids.len(); if ids.is_empty() { @@ -821,7 +826,7 @@ pub mod pallet { T::Currency::reserve(&sender, deposit)?; SuperOf::::insert(&sub, (sender.clone(), data)); - sub_ids.push(sub.clone()); + sub_ids.try_push(sub.clone()).expect("sub ids length checked above; qed"); *subs_deposit = subs_deposit.saturating_add(deposit); Self::deposit_event(Event::SubIdentityAdded(sub, sender.clone(), deposit)); diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs index d6b755df8d351..fea83dc3b10aa 100644 --- a/frame/identity/src/tests.rs +++ b/frame/identity/src/tests.rs @@ -22,7 +22,7 @@ use crate as pallet_identity; use codec::{Encode, Decode}; use sp_runtime::traits::BadOrigin; -use frame_support::{assert_ok, assert_noop, parameter_types, ord_parameter_types}; +use frame_support::{assert_ok, assert_noop, parameter_types, ord_parameter_types, BoundedVec}; use sp_core::H256; use frame_system::{EnsureSignedBy, EnsureOneOf, EnsureRoot}; use sp_runtime::{ @@ -342,14 +342,14 @@ fn setting_subaccounts_should_work() { assert_ok!(Identity::set_identity(Origin::signed(10), ten())); assert_ok!(Identity::set_subs(Origin::signed(10), subs.clone())); assert_eq!(Balances::free_balance(10), 80); - assert_eq!(Identity::subs_of(10), (10, vec![20])); + assert_eq!(Identity::subs_of(10), (10, vec![20].try_into().unwrap())); assert_eq!(Identity::super_of(20), Some((10, Data::Raw(vec![40; 1].try_into().unwrap())))); // push another item and re-set it. subs.push((30, Data::Raw(vec![50; 1].try_into().unwrap()))); assert_ok!(Identity::set_subs(Origin::signed(10), subs.clone())); assert_eq!(Balances::free_balance(10), 70); - assert_eq!(Identity::subs_of(10), (20, vec![20, 30])); + assert_eq!(Identity::subs_of(10), (20, vec![20, 30].try_into().unwrap())); assert_eq!(Identity::super_of(20), Some((10, Data::Raw(vec![40; 1].try_into().unwrap())))); assert_eq!(Identity::super_of(30), Some((10, Data::Raw(vec![50; 1].try_into().unwrap())))); @@ -357,7 +357,7 @@ fn setting_subaccounts_should_work() { subs[0] = (40, Data::Raw(vec![60; 1].try_into().unwrap())); assert_ok!(Identity::set_subs(Origin::signed(10), subs.clone())); assert_eq!(Balances::free_balance(10), 70); // no change in the balance - assert_eq!(Identity::subs_of(10), (20, vec![40, 30])); + assert_eq!(Identity::subs_of(10), (20, vec![40, 30].try_into().unwrap())); assert_eq!(Identity::super_of(20), None); assert_eq!(Identity::super_of(30), Some((10, Data::Raw(vec![50; 1].try_into().unwrap())))); assert_eq!(Identity::super_of(40), Some((10, Data::Raw(vec![60; 1].try_into().unwrap())))); @@ -365,7 +365,7 @@ fn setting_subaccounts_should_work() { // clear assert_ok!(Identity::set_subs(Origin::signed(10), vec![])); assert_eq!(Balances::free_balance(10), 90); - assert_eq!(Identity::subs_of(10), (0, vec![])); + assert_eq!(Identity::subs_of(10), (0, BoundedVec::default())); assert_eq!(Identity::super_of(30), None); assert_eq!(Identity::super_of(40), None); From 35d6fa75d231854d70e7fc57ba4ced411d19f376 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 22 Jun 2021 15:43:14 -0700 Subject: [PATCH 10/18] Add generate_storage_info --- frame/identity/src/lib.rs | 1 + frame/identity/src/types.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index 8f3bf97b5a2ef..9cd7369ae4fac 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -151,6 +151,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] + #[pallet::generate_storage_info] pub struct Pallet(_); /// Information that is pertinent to identify the entity behind an account. diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs index 44faa32ff9941..7d10886913a04 100644 --- a/frame/identity/src/types.rs +++ b/frame/identity/src/types.rs @@ -175,7 +175,7 @@ pub enum IdentityField { } /// Wrapper type for `BitFlags` that implements `Codec`. -#[derive(Clone, Copy, PartialEq, Default, RuntimeDebug)] +#[derive(Clone, Copy, PartialEq, Default, RuntimeDebug, MaxEncodedLen)] pub struct IdentityFields(pub(crate) BitFlags); impl Eq for IdentityFields {} @@ -304,7 +304,7 @@ impl< } /// Information concerning a registrar. -#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug)] +#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] pub struct RegistrarInfo< Balance: Encode + Decode + Clone + Debug + Eq + PartialEq, AccountId: Encode + Decode + Clone + Debug + Eq + PartialEq From d2291f1d0518e4854f0024f65fa667ef8d796d7f Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 23 Jun 2021 16:19:34 -0700 Subject: [PATCH 11/18] Manually implement MaxEncodedLen on select types --- frame/identity/src/types.rs | 70 +++++++++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs index 7d10886913a04..af38dd2270422 100644 --- a/frame/identity/src/types.rs +++ b/frame/identity/src/types.rs @@ -113,7 +113,7 @@ pub type RegistrarIndex = u32; /// which fields their attestation is relevant for by off-chain means. #[derive(Copy, Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen)] pub enum Judgement< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq > { /// The default value; no opinion is held. Unknown, @@ -137,7 +137,7 @@ pub enum Judgement< } impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq > Judgement { /// Returns `true` if this judgement is indicative of a deposit being currently held. This means /// it should not be cleared or replaced except by an operation which utilizes the deposit. @@ -174,10 +174,22 @@ pub enum IdentityField { Twitter = 0b0000000000000000000000000000000000000000000000000000000010000000, } +impl MaxEncodedLen for IdentityField { + fn max_encoded_len() -> usize { + 8 + } +} + /// Wrapper type for `BitFlags` that implements `Codec`. -#[derive(Clone, Copy, PartialEq, Default, RuntimeDebug, MaxEncodedLen)] +#[derive(Clone, Copy, PartialEq, Default, RuntimeDebug)] pub struct IdentityFields(pub(crate) BitFlags); +impl MaxEncodedLen for IdentityFields { + fn max_encoded_len() -> usize { + IdentityField::max_encoded_len() + } +} + impl Eq for IdentityFields {} impl Encode for IdentityFields { fn using_encoded R>(&self, f: F) -> R { @@ -195,7 +207,7 @@ impl Decode for IdentityFields { /// /// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra /// fields in a backwards compatible way through a specialized `Decode` impl. -#[derive(CloneNoBound, Encode, Decode, Eq, MaxEncodedLen, PartialEqNoBound, RuntimeDebugNoBound)] +#[derive(CloneNoBound, Encode, Decode, Eq, PartialEqNoBound, RuntimeDebugNoBound)] pub struct IdentityInfo> { /// Additional fields of the identity that are not catered for with the struct's explicit /// fields. @@ -242,6 +254,22 @@ pub struct IdentityInfo> { pub twitter: Data, } +impl> MaxEncodedLen for IdentityInfo { + fn max_encoded_len() -> usize { + let mut len = 0usize; + len = len.saturating_add(>::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); + len = len.saturating_add(>::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); + len + } +} + #[cfg(test)] impl> Default for IdentityInfo { fn default() -> Self { @@ -263,15 +291,15 @@ impl> Default for IdentityInfo { /// /// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a /// backwards compatible way through a specialized `Decode` impl. -#[derive(CloneNoBound, Encode, Eq, MaxEncodedLen, PartialEqNoBound, RuntimeDebugNoBound)] +#[derive(CloneNoBound, Encode, Eq, PartialEqNoBound, RuntimeDebugNoBound)] pub struct Registration< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, + Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgements: Get, MaxAdditionalFields: Get, > { /// Judgements from the registrars on this identity. Stored ordered by `RegistrarIndex`. There /// may be only a single judgement from each registrar. - pub judgements: BoundedVec<(RegistrarIndex, Judgement), MaxJudgments>, + pub judgements: BoundedVec<(RegistrarIndex, Judgement), MaxJudgements>, /// Amount held on deposit for this information. pub deposit: Balance, @@ -281,10 +309,24 @@ pub struct Registration< } impl < - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, - MaxJudgments: Get, + Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, + MaxJudgements: Get, + MaxAdditionalFields: Get, +> MaxEncodedLen for Registration { + fn max_encoded_len() -> usize { + let mut len = 0usize; + len = len.saturating_add(), MaxJudgements>>::max_encoded_len()); + len = len.saturating_add(Balance::max_encoded_len()); + len = len.saturating_add(>::max_encoded_len()); + len + } +} + +impl < + Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, + MaxJudgements: Get, MaxAdditionalFields: Get, -> Registration { +> Registration { pub(crate) fn total_deposit(&self) -> Balance { self.deposit + self.judgements.iter() .map(|(_, ref j)| if let Judgement::FeePaid(fee) = j { *fee } else { Zero::zero() }) @@ -293,10 +335,10 @@ impl < } impl< - Balance: Encode + Decode + Copy + Clone + Debug + Eq + PartialEq, - MaxJudgments: Get, + Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq, + MaxJudgements: Get, MaxAdditionalFields: Get, -> Decode for Registration { +> Decode for Registration { fn decode(input: &mut I) -> sp_std::result::Result { let (judgements, deposit, info) = Decode::decode(&mut AppendZerosInput::new(input))?; Ok(Self { judgements, deposit, info }) From cd87e2489b98f3857cc36a8d3e8f4fe51de92ca2 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 23 Jun 2021 19:52:38 -0700 Subject: [PATCH 12/18] Use ConstU32 instead of parameter_type --- frame/identity/src/types.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs index af38dd2270422..1fc75c5b865ea 100644 --- a/frame/identity/src/types.rs +++ b/frame/identity/src/types.rs @@ -18,8 +18,7 @@ use codec::{Encode, Decode}; use enumflags2::BitFlags; use frame_support::{ - parameter_types, - traits::{Get, MaxEncodedLen}, + traits::{ConstU32, Get, MaxEncodedLen}, BoundedVec, CloneNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; use sp_std::prelude::*; @@ -30,10 +29,6 @@ use sp_runtime::{ }; use super::*; -parameter_types! { - pub const MaxDataSize: u32 = 32; -} - /// Either underlying data blob if it is at most 32 bytes, or a hash of it. If the data is greater /// than 32-bytes then it will be truncated when encoding. /// @@ -43,7 +38,7 @@ pub enum Data { /// No data here. None, /// The data is stored directly. - Raw(BoundedVec), + Raw(BoundedVec>), /// Only the Blake2 hash of the data is stored. The preimage of the hash may be retrieved /// through some hash-lookup service. BlakeTwo256([u8; 32]), From cab319356e75b5beedbc7dfe905b4ebb0af90c0b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 23 Jun 2021 20:01:38 -0700 Subject: [PATCH 13/18] Leverage DefaultNoBound and add some comments --- frame/identity/src/types.rs | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs index 1fc75c5b865ea..9e35b92e67a3b 100644 --- a/frame/identity/src/types.rs +++ b/frame/identity/src/types.rs @@ -203,6 +203,7 @@ impl Decode for IdentityFields { /// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra /// fields in a backwards compatible way through a specialized `Decode` impl. #[derive(CloneNoBound, Encode, Decode, Eq, PartialEqNoBound, RuntimeDebugNoBound)] +#[cfg_attr(test, derive(frame_support::DefaultNoBound))] pub struct IdentityInfo> { /// Additional fields of the identity that are not catered for with the struct's explicit /// fields. @@ -253,35 +254,18 @@ impl> MaxEncodedLen for IdentityInfo { fn max_encoded_len() -> usize { let mut len = 0usize; len = len.saturating_add(>::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); - len = len.saturating_add(>::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); + len = len.saturating_add(Data::max_encoded_len()); // display + len = len.saturating_add(Data::max_encoded_len()); // legal + len = len.saturating_add(Data::max_encoded_len()); // web + len = len.saturating_add(Data::max_encoded_len()); // riot + len = len.saturating_add(Data::max_encoded_len()); // email + len = len.saturating_add(>::max_encoded_len()); // PGP fingerprint + len = len.saturating_add(Data::max_encoded_len()); // image + len = len.saturating_add(Data::max_encoded_len()); // twitter len } } -#[cfg(test)] -impl> Default for IdentityInfo { - fn default() -> Self { - Self { - additional: Default::default(), - display: Default::default(), - legal: Default::default(), - web: Default::default(), - riot: Default::default(), - email: Default::default(), - pgp_fingerprint: Default::default(), - image: Default::default(), - twitter: Default::default(), - } - } -} - /// Information concerning the identity of the controller of an account. /// /// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a From 78814b67253f63555be988bacf3b981eb4334f3b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 24 Jun 2021 14:52:55 -0700 Subject: [PATCH 14/18] Use max_encoded_len() instead of hardcoded constant --- frame/identity/src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs index 9e35b92e67a3b..c4bc19edccea8 100644 --- a/frame/identity/src/types.rs +++ b/frame/identity/src/types.rs @@ -171,7 +171,7 @@ pub enum IdentityField { impl MaxEncodedLen for IdentityField { fn max_encoded_len() -> usize { - 8 + u64::max_encoded_len() } } From c892b62fcc95941a357d094d82ee8bb67207ae9b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 5 Jul 2021 22:23:48 -0700 Subject: [PATCH 15/18] Use MaxEncodedLen in parity-scal-codec --- Cargo.lock | 1 - frame/identity/Cargo.toml | 4 +--- frame/identity/src/types.rs | 44 +++++++++---------------------------- 3 files changed, 11 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e0239a481fce5..8ca21d1d9056f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5117,7 +5117,6 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "max-encoded-len", "pallet-balances", "parity-scale-codec", "sp-core", diff --git a/frame/identity/Cargo.toml b/frame/identity/Cargo.toml index 0438183612b10..ed905d407d90f 100644 --- a/frame/identity/Cargo.toml +++ b/frame/identity/Cargo.toml @@ -13,7 +13,7 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } +codec = { package = "parity-scale-codec", version = "2.2.0", default-features = false, features = ["derive", "max-encoded-len"] } enumflags2 = { version = "0.6.2" } sp-std = { version = "3.0.0", default-features = false, path = "../../primitives/std" } sp-io = { version = "3.0.0", default-features = false, path = "../../primitives/io" } @@ -21,7 +21,6 @@ sp-runtime = { version = "3.0.0", default-features = false, path = "../../primit frame-benchmarking = { version = "3.1.0", default-features = false, path = "../benchmarking", optional = true } frame-support = { version = "3.0.0", default-features = false, path = "../support" } frame-system = { version = "3.0.0", default-features = false, path = "../system" } -max-encoded-len = { version = "3.0.0", default-features = false, path = "../../max-encoded-len", features = ["derive"] } [dev-dependencies] sp-core = { version = "3.0.0", path = "../../primitives/core" } @@ -37,7 +36,6 @@ std = [ "frame-benchmarking/std", "frame-support/std", "frame-system/std", - "max-encoded-len/std", ] runtime-benchmarks = ["frame-benchmarking"] try-runtime = ["frame-support/try-runtime"] diff --git a/frame/identity/src/types.rs b/frame/identity/src/types.rs index def1b8fec2bca..59781aadbd31e 100644 --- a/frame/identity/src/types.rs +++ b/frame/identity/src/types.rs @@ -15,10 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use codec::{Encode, Decode}; +use codec::{Encode, Decode, MaxEncodedLen}; use enumflags2::BitFlags; use frame_support::{ - traits::{ConstU32, Get, MaxEncodedLen}, + traits::{ConstU32, Get}, BoundedVec, CloneNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; use sp_std::prelude::*; @@ -202,7 +202,8 @@ impl Decode for IdentityFields { /// /// NOTE: This should be stored at the end of the storage item to facilitate the addition of extra /// fields in a backwards compatible way through a specialized `Decode` impl. -#[derive(CloneNoBound, Encode, Decode, Eq, PartialEqNoBound, RuntimeDebugNoBound)] +#[derive(CloneNoBound, Encode, Decode, Eq, MaxEncodedLen, PartialEqNoBound, RuntimeDebugNoBound)] +#[codec(mel_bound(FieldLimit: Get))] #[cfg_attr(test, derive(frame_support::DefaultNoBound))] pub struct IdentityInfo> { /// Additional fields of the identity that are not catered for with the struct's explicit @@ -250,27 +251,16 @@ pub struct IdentityInfo> { pub twitter: Data, } -impl> MaxEncodedLen for IdentityInfo { - fn max_encoded_len() -> usize { - let mut len = 0usize; - len = len.saturating_add(>::max_encoded_len()); - len = len.saturating_add(Data::max_encoded_len()); // display - len = len.saturating_add(Data::max_encoded_len()); // legal - len = len.saturating_add(Data::max_encoded_len()); // web - len = len.saturating_add(Data::max_encoded_len()); // riot - len = len.saturating_add(Data::max_encoded_len()); // email - len = len.saturating_add(>::max_encoded_len()); // PGP fingerprint - len = len.saturating_add(Data::max_encoded_len()); // image - len = len.saturating_add(Data::max_encoded_len()); // twitter - len - } -} - /// Information concerning the identity of the controller of an account. /// /// NOTE: This is stored separately primarily to facilitate the addition of extra fields in a /// backwards compatible way through a specialized `Decode` impl. -#[derive(CloneNoBound, Encode, Eq, PartialEqNoBound, RuntimeDebugNoBound)] +#[derive(CloneNoBound, Encode, Eq, MaxEncodedLen, PartialEqNoBound, RuntimeDebugNoBound)] +#[codec(mel_bound( + Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, + MaxJudgements: Get, + MaxAdditionalFields: Get, +))] pub struct Registration< Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq, MaxJudgements: Get, @@ -287,20 +277,6 @@ pub struct Registration< pub info: IdentityInfo, } -impl < - Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, - MaxJudgements: Get, - MaxAdditionalFields: Get, -> MaxEncodedLen for Registration { - fn max_encoded_len() -> usize { - let mut len = 0usize; - len = len.saturating_add(), MaxJudgements>>::max_encoded_len()); - len = len.saturating_add(Balance::max_encoded_len()); - len = len.saturating_add(>::max_encoded_len()); - len - } -} - impl < Balance: Encode + Decode + MaxEncodedLen + Copy + Clone + Debug + Eq + PartialEq + Zero + Add, MaxJudgements: Get, From 36d917c5c50ccfe741397dcb8b5991742281e90b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 6 Jul 2021 23:42:37 -0700 Subject: [PATCH 16/18] Add get_mut method for WeakBoundedVec --- frame/support/src/storage/weak_bounded_vec.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frame/support/src/storage/weak_bounded_vec.rs b/frame/support/src/storage/weak_bounded_vec.rs index e5a4843000bbf..a98d2182d0919 100644 --- a/frame/support/src/storage/weak_bounded_vec.rs +++ b/frame/support/src/storage/weak_bounded_vec.rs @@ -89,6 +89,14 @@ impl WeakBoundedVec { pub fn retain bool>(&mut self, f: F) { self.0.retain(f) } + + /// Exactly the same semantics as [`Vec::get_mut`]. + pub fn get_mut>( + &mut self, + index: I, + ) -> Option<&mut >::Output> { + self.0.get_mut(index) + } } impl> WeakBoundedVec { From 263c3f475add1ef7f152c3d74b624bf3e33bfcac Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 7 Jul 2021 17:23:09 -0700 Subject: [PATCH 17/18] Use expect on an infallible operation --- frame/identity/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index 332142883f4e4..b3306b3b91b82 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -427,7 +427,7 @@ pub mod pallet { }) .collect::>() .try_into() - .map_err(|_| Error::::TooManySubAccounts)?; + .expect("subs is less than T::MaxSubAccounts; qed"); let new_subs = ids.len(); if ids.is_empty() { From 8d30b525643d52b5df393d70326fc81fe5c82fd7 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 7 Jul 2021 17:28:49 -0700 Subject: [PATCH 18/18] Rewrite as for loop --- frame/identity/src/lib.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index b3306b3b91b82..f6e3f0639f16a 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -419,15 +419,11 @@ pub mod pallet { for s in old_ids.iter() { >::remove(s); } - let ids: BoundedVec<_, _> = subs - .into_iter() - .map(|(id, name)| { - >::insert(&id, (sender.clone(), name)); - id - }) - .collect::>() - .try_into() - .expect("subs is less than T::MaxSubAccounts; qed"); + let mut ids = BoundedVec::::default(); + for (id, name) in subs { + >::insert(&id, (sender.clone(), name)); + ids.try_push(id).expect("subs length is less than T::MaxSubAccounts; qed"); + } let new_subs = ids.len(); if ids.is_empty() {