From 8685766d6f72b8502d9528910c9066042494f578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89lo=C3=AFs?= <c@elo.tf> Date: Tue, 12 Jul 2022 21:47:43 +0200 Subject: [PATCH] feat(identity): add call change_owner_key (nodes/rust/duniter-v2s!86) * feat(identity): add call change_owner_key --- Cargo.lock | 2 +- node/src/chain_spec/gdev.rs | 6 +- pallets/authority-members/src/lib.rs | 71 +++++++---- pallets/duniter-wot/src/lib.rs | 9 +- pallets/duniter-wot/src/mock.rs | 7 +- pallets/duniter-wot/src/tests.rs | 3 +- pallets/identity/Cargo.toml | 37 ++---- pallets/identity/src/lib.rs | 183 +++++++++++++++++++-------- pallets/identity/src/mock.rs | 4 + pallets/identity/src/tests.rs | 149 ++++++++++++++++++++-- pallets/identity/src/traits.rs | 15 ++- pallets/identity/src/types.rs | 20 ++- runtime/common/src/entities.rs | 23 ++++ runtime/common/src/handlers.rs | 31 +++++ runtime/common/src/pallets_config.rs | 5 +- runtime/g1/src/parameters.rs | 1 + runtime/gdev/src/parameters.rs | 9 ++ runtime/gdev/tests/common/mod.rs | 3 +- runtime/gtest/src/parameters.rs | 1 + 19 files changed, 441 insertions(+), 138 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2bb67beb8..62a887f91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5240,7 +5240,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "maplit", + "impl-trait-for-tuples", "parity-scale-codec", "scale-info", "serde", diff --git a/node/src/chain_spec/gdev.rs b/node/src/chain_spec/gdev.rs index 6e51deef8..b31c4f7da 100644 --- a/node/src/chain_spec/gdev.rs +++ b/node/src/chain_spec/gdev.rs @@ -374,11 +374,12 @@ fn gen_genesis_for_local_chain( index: i as u32 + 1, name: name.clone(), value: IdtyValue { + data: IdtyData::new(), next_creatable_identity_on: Default::default(), + old_owner_key: None, owner_key: owner_key.clone(), removable_on: 0, status: IdtyStatus::Validated, - data: IdtyData::new(), }, }) .collect(), @@ -500,11 +501,12 @@ fn genesis_data_to_gdev_genesis_conf( index: i as u32 + 1, name: common_runtime::IdtyName::from(name.as_str()), value: common_runtime::IdtyValue { + data: IdtyData::new(), next_creatable_identity_on: 0, + old_owner_key: None, owner_key: pubkey, removable_on: 0, status: IdtyStatus::Validated, - data: IdtyData::new(), }, }) .collect(), diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs index 987962f47..2238d8722 100644 --- a/pallets/authority-members/src/lib.rs +++ b/pallets/authority-members/src/lib.rs @@ -100,7 +100,6 @@ pub mod pallet { impl<T: Config> GenesisBuild<T> for GenesisConfig<T> { fn build(&self) { for (member_id, (account_id, _is_online)) in &self.initial_authorities { - AccountIdOf::<T>::insert(member_id, account_id); Members::<T>::insert( member_id, MemberData::new_genesis(T::MaxKeysLife::get(), account_id.to_owned()), @@ -255,7 +254,7 @@ pub mod pallet { if !Members::<T>::contains_key(member_id) { return Err(Error::<T>::MemberNotFound.into()); } - let validator_id = T::ValidatorIdOf::convert(who.clone()) + let validator_id = T::ValidatorIdOf::convert(who) .ok_or(pallet_session::Error::<T>::NoAssociatedValidatorId)?; if !pallet_session::Pallet::<T>::is_registered(&validator_id) { return Err(Error::<T>::SessionKeysNotProvided.into()); @@ -276,7 +275,7 @@ pub mod pallet { if is_outgoing { Self::remove_out(member_id); } else { - Self::insert_in(member_id, who); + Self::insert_in(member_id); } Ok(().into()) @@ -315,29 +314,54 @@ pub mod pallet { Ok(().into()) } #[pallet::weight(1_000_000_000)] - pub fn prune_account_id_of( + pub fn remove_member( origin: OriginFor<T>, - members_ids: Vec<T::MemberId>, + member_id: T::MemberId, ) -> DispatchResultWithPostInfo { - ensure_root(origin)?; + T::RemoveMemberOrigin::ensure_origin(origin)?; - for member_id in members_ids { - if !Self::is_online(member_id) && !Self::is_incoming(member_id) { - AccountIdOf::<T>::remove(member_id); - } - } + let member_data = Members::<T>::get(member_id).ok_or(Error::<T>::NotMember)?; + Self::do_remove_member(member_id, member_data.owner_key); Ok(().into()) } - #[pallet::weight(1_000_000_000)] - pub fn remove_member( - origin: OriginFor<T>, + } + + // PUBLIC FUNCTIONS // + + impl<T: Config> Pallet<T> { + /// Should be transactional + #[frame_support::transactional] + pub fn change_owner_key( member_id: T::MemberId, + new_owner_key: T::AccountId, ) -> DispatchResultWithPostInfo { - T::RemoveMemberOrigin::ensure_origin(origin)?; + let old_owner_key = Members::<T>::mutate_exists(member_id, |maybe_member_data| { + if let Some(ref mut member_data) = maybe_member_data { + Ok(core::mem::replace( + &mut member_data.owner_key, + new_owner_key.clone(), + )) + } else { + Err(Error::<T>::MemberNotFound) + } + })?; - let member_data = Members::<T>::get(member_id).ok_or(Error::<T>::NotMember)?; - Self::do_remove_member(member_id, member_data.owner_key); + let validator_id = T::ValidatorIdOf::convert(old_owner_key.clone()) + .ok_or(Error::<T>::MemberNotFound)?; + let session_keys = pallet_session::NextKeys::<T>::get(validator_id) + .ok_or(Error::<T>::SessionKeysNotProvided)?; + + // Purge session keys + let _post_info = pallet_session::Call::<T>::purge_keys {} + .dispatch_bypass_filter(frame_system::RawOrigin::Signed(old_owner_key).into())?; + + // Set session keys + let _post_info = pallet_session::Call::<T>::set_keys { + keys: session_keys, + proof: vec![], + } + .dispatch_bypass_filter(frame_system::RawOrigin::Signed(new_owner_key).into())?; Ok(().into()) } @@ -390,7 +414,7 @@ pub mod pallet { } } } - fn insert_in(member_id: T::MemberId, account_id: T::AccountId) -> bool { + fn insert_in(member_id: T::MemberId) -> bool { let not_already_inserted = IncomingAuthorities::<T>::mutate(|members_ids| { if let Err(index) = members_ids.binary_search(&member_id) { members_ids.insert(index, member_id); @@ -401,7 +425,6 @@ pub mod pallet { }); if not_already_inserted { AuthoritiesCounter::<T>::mutate(|counter| *counter += 1); - AccountIdOf::<T>::insert(member_id, account_id); Self::deposit_event(Event::MemberGoOnline(member_id)); } not_already_inserted @@ -510,8 +533,6 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> { } }); MembersExpireOn::<T>::append(expire_on_session, member_id); - // Prune AccountIdOf - AccountIdOf::<T>::remove(member_id); } Self::deposit_event(Event::OutgoingAuthorities(members_ids_to_del.clone())); } @@ -535,8 +556,8 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> { }) .into_iter() .filter_map(|member_id| { - if let Some(account_id) = AccountIdOf::<T>::get(member_id) { - T::ValidatorIdOf::convert(account_id) + if let Some(member_data) = Members::<T>::get(member_id) { + T::ValidatorIdOf::convert(member_data.owner_key) } else { None } @@ -550,8 +571,8 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> { OnlineAuthorities::<T>::get() .into_iter() .filter_map(|member_id| { - if let Some(account_id) = AccountIdOf::<T>::get(member_id) { - T::ValidatorIdOf::convert(account_id) + if let Some(member_data) = Members::<T>::get(member_id) { + T::ValidatorIdOf::convert(member_data.owner_key) } else { None } diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs index 693b30dd7..2b4573324 100644 --- a/pallets/duniter-wot/src/lib.rs +++ b/pallets/duniter-wot/src/lib.rs @@ -233,12 +233,12 @@ where } impl<T: Config<I>, I: 'static> pallet_identity::traits::OnIdtyChange<T> for Pallet<T, I> { - fn on_idty_change(idty_index: IdtyIndex, idty_event: IdtyEvent<T>) -> Weight { + fn on_idty_change(idty_index: IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight { match idty_event { IdtyEvent::Created { creator } => { if let Err(e) = <pallet_certification::Pallet<T, I>>::force_add_cert( frame_system::Origin::<T>::Root.into(), - creator, + *creator, idty_index, true, ) { @@ -247,10 +247,8 @@ impl<T: Config<I>, I: 'static> pallet_identity::traits::OnIdtyChange<T> for Pall } } } - IdtyEvent::Confirmed => {} - IdtyEvent::Validated => {} IdtyEvent::Removed { status } => { - if status != IdtyStatus::Validated { + if *status != IdtyStatus::Validated { if let Err(e) = <pallet_certification::Pallet<T, I>>::remove_all_certs_received_by( frame_system::Origin::<T>::Root.into(), @@ -263,6 +261,7 @@ impl<T: Config<I>, I: 'static> pallet_identity::traits::OnIdtyChange<T> for Pall } } } + IdtyEvent::Confirmed | IdtyEvent::Validated | IdtyEvent::ChangedOwnerKey { .. } => {} } 0 } diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index 5646650d9..9a8be5817 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -96,6 +96,7 @@ impl pallet_duniter_wot::Config<Instance1> for Test { // Identity parameter_types! { + pub const ChangeOwnerKeyPeriod: u64 = 10; pub const ConfirmPeriod: u64 = 2; pub const IdtyCreationPeriod: u64 = 3; pub const ValidationPeriod: u64 = 2; @@ -109,6 +110,7 @@ impl pallet_identity::traits::IdtyNameValidator for IdtyNameValidatorTestImpl { } impl pallet_identity::Config for Test { + type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod; type ConfirmPeriod = ConfirmPeriod; type Event = Event; type EnsureIdtyCallAllowed = DuniterWot; @@ -118,6 +120,8 @@ impl pallet_identity::Config for Test { type IdtyIndex = IdtyIndex; type IdtyValidationOrigin = system::EnsureRoot<AccountId>; type IsMember = Membership; + type NewOwnerKeySigner = UintAuthorityId; + type NewOwnerKeySignature = TestSignature; type OnIdtyChange = DuniterWot; type RemoveIdentityConsumers = (); type RevocationSigner = UintAuthorityId; @@ -245,11 +249,12 @@ pub fn new_test_ext( index: i as u32, name: pallet_identity::IdtyName::from(NAMES[i - 1]), value: pallet_identity::IdtyValue { + data: (), next_creatable_identity_on: 0, owner_key: i as u64, + old_owner_key: None, removable_on: 0, status: pallet_identity::IdtyStatus::Validated, - data: (), }, }) .collect(), diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs index 870f29ccd..4a7125785 100644 --- a/pallets/duniter-wot/src/tests.rs +++ b/pallets/duniter-wot/src/tests.rs @@ -213,11 +213,12 @@ fn test_new_idty_validation() { assert_eq!( Identity::identity(6), Some(pallet_identity::IdtyValue { + data: (), next_creatable_identity_on: 0, + old_owner_key: None, owner_key: 6, removable_on: 0, status: IdtyStatus::Validated, - data: () }) ); }); diff --git a/pallets/identity/Cargo.toml b/pallets/identity/Cargo.toml index 9fc3fc9e6..993109141 100644 --- a/pallets/identity/Cargo.toml +++ b/pallets/identity/Cargo.toml @@ -24,16 +24,18 @@ std = [ ] try-runtime = ['frame-support/try-runtime'] +[package.metadata.docs.rs] +targets = ['x86_64-unknown-linux-gnu'] + [dependencies] -# substrate +# crates.io +codec = { package = 'parity-scale-codec', version = '2.3.1', features = ['derive'], default-features = false } +impl-trait-for-tuples = "0.2.1" scale-info = { version = "1.0", default-features = false, features = ["derive"] } +serde = { version = "1.0.101", features = ["derive"], optional = true } -[dependencies.codec] -default-features = false -features = ['derive'] -package = 'parity-scale-codec' -version = '2.3.1' +# substrate [dependencies.frame-benchmarking] default-features = false @@ -51,11 +53,6 @@ default-features = false git = 'https://github.com/librelois/substrate.git' branch = 'duniter-monthly-2022-02' -[dependencies.serde] -version = "1.0.101" -optional = true -features = ["derive"] - [dependencies.sp-core] default-features = false git = 'https://github.com/librelois/substrate.git' @@ -71,18 +68,6 @@ default-features = false git = 'https://github.com/librelois/substrate.git' branch = 'duniter-monthly-2022-02' -### DOC ### - -[package.metadata.docs.rs] -targets = ['x86_64-unknown-linux-gnu'] -[dev-dependencies.serde] -version = '1.0.119' - -### DEV ### - -[dev-dependencies.maplit] -version = '1.0.2' - -[dev-dependencies.sp-io] -git = 'https://github.com/librelois/substrate.git' -branch = 'duniter-monthly-2022-02' +[dev-dependencies] +serde = '1.0.119' +sp-io = { git = 'https://github.com/librelois/substrate.git', branch = 'duniter-monthly-2022-02' } diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index 0cc9fc089..5c54f9495 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -39,6 +39,9 @@ use sp_runtime::traits::{AtLeast32BitUnsigned, IdentifyAccount, One, Saturating, use sp_std::fmt::Debug; use sp_std::prelude::*; +pub const NEW_OWNER_KEY_PAYLOAD_PREFIX: [u8; 4] = [b'i', b'c', b'o', b'k']; +pub const REVOCATION_PAYLOAD_PREFIX: [u8; 4] = [b'r', b'e', b'v', b'o']; + #[frame_support::pallet] pub mod pallet { use super::*; @@ -62,6 +65,9 @@ pub mod pallet { #[pallet::constant] /// Period during which the owner can confirm the new identity. type ConfirmPeriod: Get<Self::BlockNumber>; + #[pallet::constant] + /// Minimum duration between two owner key changes + type ChangeOwnerKeyPeriod: Get<Self::BlockNumber>; /// Because this pallet emits events, it depends on the runtime's definition of an event. type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>; /// Management of the authorizations of the different calls. (The default implementation only allows root) @@ -94,6 +100,10 @@ pub mod pallet { type IsMember: sp_runtime::traits::IsMember<Self::IdtyIndex>; /// On identity confirmed by its owner type OnIdtyChange: OnIdtyChange<Self>; + /// Signing key of new owner key payload + type NewOwnerKeySigner: IdentifyAccount<AccountId = Self::AccountId>; + /// Signature of new owner key payload + type NewOwnerKeySignature: Parameter + Verify<Signer = Self::NewOwnerKeySigner>; /// Handle the logic that removes all identity consumers. /// "identity consumers" meaning all things that rely on the existence of the identity. type RemoveIdentityConsumers: RemoveIdentityConsumers<Self::IdtyIndex>; @@ -225,6 +235,10 @@ pub mod pallet { /// An identity has been validated /// [idty_index] IdtyValidated { idty_index: T::IdtyIndex }, + IdtyChangedOwnerKey { + idty_index: T::IdtyIndex, + new_owner_key: T::AccountId, + }, /// An identity has been removed /// [idty_index] IdtyRemoved { idty_index: T::IdtyIndex }, @@ -284,11 +298,12 @@ pub mod pallet { <Identities<T>>::insert( idty_index, IdtyValue { + data: Default::default(), next_creatable_identity_on: T::BlockNumber::zero(), + old_owner_key: None, owner_key: owner_key.clone(), removable_on, status: IdtyStatus::Created, - data: Default::default(), }, ); IdentitiesRemovableOn::<T>::append(removable_on, (idty_index, IdtyStatus::Created)); @@ -297,7 +312,7 @@ pub mod pallet { idty_index, owner_key, }); - T::OnIdtyChange::on_idty_change(idty_index, IdtyEvent::Created { creator }); + T::OnIdtyChange::on_idty_change(idty_index, &IdtyEvent::Created { creator }); Ok(().into()) } /// Confirm the creation of an identity and give it a name @@ -342,7 +357,7 @@ pub mod pallet { owner_key: who, name: idty_name, }); - T::OnIdtyChange::on_idty_change(idty_index, IdtyEvent::Confirmed); + T::OnIdtyChange::on_idty_change(idty_index, &IdtyEvent::Confirmed); Ok(().into()) } #[pallet::weight(1_000_000_000)] @@ -372,40 +387,118 @@ pub mod pallet { <Identities<T>>::insert(idty_index, idty_value); Self::deposit_event(Event::IdtyValidated { idty_index }); - T::OnIdtyChange::on_idty_change(idty_index, IdtyEvent::Validated); + T::OnIdtyChange::on_idty_change(idty_index, &IdtyEvent::Validated); Ok(().into()) } - /// Revoke an identity using a signed revocation payload + + /// Change identity owner key. /// - /// - `payload`: the revocation payload - /// - `owner_key`: the public key corresponding to the identity to be revoked - /// - `genesis_hash`: the genesis block hash - /// - `payload_sig`: the signature of the encoded form of `payload`. Must be signed by `owner_key`. + /// - `new_key`: the new owner key. + /// - `new_key_sig`: the signature of the encoded form of `NewOwnerKeyPayload`. + /// Must be signed by `new_key`. /// - /// Any origin can emit this extrinsic, not only `owner_key`. + /// The origin should be the old identity owner key. + #[pallet::weight(1_000_000_000)] + pub fn change_owner_key( + origin: OriginFor<T>, + new_key: T::AccountId, + new_key_sig: T::NewOwnerKeySignature, + ) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + + let idty_index = + IdentityIndexOf::<T>::get(&who).ok_or(Error::<T>::IdtyIndexNotFound)?; + let mut idty_value = + Identities::<T>::get(idty_index).ok_or(Error::<T>::IdtyNotFound)?; + + ensure!( + IdentityIndexOf::<T>::get(&new_key).is_none(), + Error::<T>::OwnerKeyAlreadyUsed + ); + + let block_number = frame_system::Pallet::<T>::block_number(); + if let Some((_, last_change)) = idty_value.old_owner_key { + ensure!( + block_number >= last_change + T::ChangeOwnerKeyPeriod::get(), + Error::<T>::OwnerKeyAlreadyRecentlyChanged + ); + } + + let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); + let new_key_payload = NewOwnerKeyPayload { + genesis_hash: &genesis_hash, + idty_index, + old_owner_key: &idty_value.owner_key, + }; + + ensure!( + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload) + .using_encoded(|bytes| new_key_sig.verify(bytes, &new_key)), + Error::<T>::InvalidNewOwnerKeySig + ); + + IdentityIndexOf::<T>::remove(&idty_value.owner_key); + idty_value.old_owner_key = Some((idty_value.owner_key.clone(), block_number)); + idty_value.owner_key = new_key.clone(); + IdentityIndexOf::<T>::insert(&idty_value.owner_key, idty_index); + Identities::<T>::insert(idty_index, idty_value); + Self::deposit_event(Event::IdtyChangedOwnerKey { + idty_index, + new_owner_key: new_key.clone(), + }); + T::OnIdtyChange::on_idty_change( + idty_index, + &IdtyEvent::ChangedOwnerKey { + new_owner_key: new_key, + }, + ); + + Ok(().into()) + } + + /// Revoke an identity using a revocation signature + /// + /// - `idty_index`: the index of the identity to be revoked. + /// - `revocation_key`: the key used to sign the revocation payload. + /// - `revocation_sig`: the signature of the encoded form of `RevocationPayload`. + /// Must be signed by `revocation_key`. + /// + /// Any signed origin can execute this call. #[pallet::weight(1_000_000_000)] pub fn revoke_identity( origin: OriginFor<T>, - payload: RevocationPayload<T::AccountId, T::Hash>, - payload_sig: T::RevocationSignature, + idty_index: T::IdtyIndex, + revocation_key: T::AccountId, + revocation_sig: T::RevocationSignature, ) -> DispatchResultWithPostInfo { let _ = ensure_signed(origin)?; + + let idty_value = Identities::<T>::get(idty_index).ok_or(Error::<T>::IdtyNotFound)?; + ensure!( - payload.genesis_hash - == frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()), - Error::<T>::InvalidGenesisHash + if let Some((ref old_owner_key, _)) = idty_value.old_owner_key { + revocation_key == idty_value.owner_key || &revocation_key == old_owner_key + } else { + revocation_key == idty_value.owner_key + }, + Error::<T>::InvalidRevocationKey ); + + let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); + let revocation_payload = RevocationPayload { + genesis_hash, + idty_index, + }; + ensure!( - payload.using_encoded(|bytes| payload_sig.verify(bytes, &payload.owner_key)), - Error::<T>::InvalidRevocationProof + (REVOCATION_PAYLOAD_PREFIX, revocation_payload) + .using_encoded(|bytes| revocation_sig.verify(bytes, &idty_value.owner_key)), + Error::<T>::InvalidRevocationSig ); - if let Some(idty_index) = <IdentityIndexOf<T>>::take(&payload.owner_key) { - Self::do_remove_identity(idty_index); - Ok(().into()) - } else { - Err(Error::<T>::IdtyNotFound.into()) - } + + Self::do_remove_identity(idty_index); + Ok(().into()) } #[pallet::weight(1_000_000_000)] @@ -437,37 +530,12 @@ pub mod pallet { Ok(().into()) } - - #[pallet::weight(1_000_000_000)] - pub fn prune_item_identity_index_of( - origin: OriginFor<T>, - accounts_ids: Vec<T::AccountId>, - ) -> DispatchResultWithPostInfo { - ensure_root(origin)?; - - accounts_ids - .into_iter() - .filter(|account_id| { - if let Ok(idty_index) = IdentityIndexOf::<T>::try_get(&account_id) { - !Identities::<T>::contains_key(&idty_index) - } else { - false - } - }) - .for_each(IdentityIndexOf::<T>::remove); - - Ok(().into()) - } } // ERRORS // #[pallet::error] pub enum Error<T> { - /// Genesis hash does not match - InvalidGenesisHash, - /// Revocation payload signature is invalid - InvalidRevocationProof, /// Creator not allowed to create identities CreatorNotAllowedToCreateIdty, /// Identity already confirmed @@ -494,21 +562,28 @@ pub mod pallet { IdtyNotValidated, /// Identity not yet renewable IdtyNotYetRenewable, + /// New owner key payload signature is invalid + InvalidNewOwnerKeySig, + /// Revocation key is invalid + InvalidRevocationKey, + /// Revocation payload signature is invalid + InvalidRevocationSig, /// Not allowed to confirm identity NotAllowedToConfirmIdty, /// Not allowed to validate identity NotAllowedToValidateIdty, + /// Identity creation period is not respected + NotRespectIdtyCreationPeriod, /// Not the same identity name NotSameIdtyName, + /// Owner key already recently changed + OwnerKeyAlreadyRecentlyChanged, + /// Owner key already used + OwnerKeyAlreadyUsed, /// Right already added RightAlreadyAdded, /// Right does not exist RightNotExist, - /// Identity creation period is not respected - NotRespectIdtyCreationPeriod, - // Removed error:Â OwnerAccountNotExist, - // Caution: if you add a new error, you should explicitly set the index - // to not reuse the same index as the removed error. } // PUBLIC FUNCTIONS // @@ -532,7 +607,7 @@ pub mod pallet { Self::deposit_event(Event::IdtyRemoved { idty_index }); T::OnIdtyChange::on_idty_change( idty_index, - IdtyEvent::Removed { + &IdtyEvent::Removed { status: idty_val.status, }, ); diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index 36e24dca7..2e34e7494 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -76,6 +76,7 @@ impl system::Config for Test { } parameter_types! { + pub const ChangeOwnerKeyPeriod: u64 = 10; pub const ConfirmPeriod: u64 = 2; pub const IdtyCreationPeriod: u64 = 3; pub const MaxInactivityPeriod: u64 = 5; @@ -98,6 +99,7 @@ impl IsMember<u64> for IsMemberTestImpl { } impl pallet_identity::Config for Test { + type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod; type ConfirmPeriod = ConfirmPeriod; type Event = Event; type EnsureIdtyCallAllowed = (); @@ -107,6 +109,8 @@ impl pallet_identity::Config for Test { type IdtyIndex = u64; type IdtyValidationOrigin = system::EnsureRoot<AccountId>; type IsMember = IsMemberTestImpl; + type NewOwnerKeySigner = UintAuthorityId; + type NewOwnerKeySignature = TestSignature; type OnIdtyChange = (); type RemoveIdentityConsumers = (); type RevocationSigner = UintAuthorityId; diff --git a/pallets/identity/src/tests.rs b/pallets/identity/src/tests.rs index f2f50efb1..be17d9068 100644 --- a/pallets/identity/src/tests.rs +++ b/pallets/identity/src/tests.rs @@ -15,10 +15,13 @@ // along with Substrate-Libre-Currency. If not, see <https://www.gnu.org/licenses/>. use crate::mock::*; -use crate::{Error, GenesisIdty, IdtyName, IdtyValue, RevocationPayload}; +use crate::{ + Error, GenesisIdty, IdtyName, IdtyValue, NewOwnerKeyPayload, RevocationPayload, + NEW_OWNER_KEY_PAYLOAD_PREFIX, REVOCATION_PAYLOAD_PREFIX, +}; use codec::Encode; //use frame_support::assert_noop; -use frame_support::assert_ok; +use frame_support::{assert_err, assert_ok}; use frame_system::{EventRecord, Phase}; use sp_runtime::testing::TestSignature; @@ -29,11 +32,27 @@ fn alice() -> GenesisIdty<Test> { index: 1, name: IdtyName::from("Alice"), value: IdtyVal { + data: (), next_creatable_identity_on: 0, + old_owner_key: None, owner_key: 1, removable_on: 0, status: crate::IdtyStatus::Validated, + }, + } +} + +fn bob() -> GenesisIdty<Test> { + GenesisIdty { + index: 2, + name: IdtyName::from("Bob"), + value: IdtyVal { data: (), + next_creatable_identity_on: 0, + old_owner_key: None, + owner_key: 2, + removable_on: 0, + status: crate::IdtyStatus::Validated, }, } } @@ -172,34 +191,139 @@ fn test_idty_creation_period() { } #[test] -fn test_idty_revocation() { +fn test_change_owner_key() { new_test_ext(IdentityConfig { - identities: vec![alice()], + identities: vec![alice(), bob()], }) .execute_with(|| { + let genesis_hash = System::block_hash(0); + let old_owner_key = 1u64; + let new_key_payload = NewOwnerKeyPayload { + genesis_hash: &genesis_hash, + idty_index: 1u64, + old_owner_key: &old_owner_key, + }; + // We need to initialize at least one block before any call run_to_block(1); + // Caller should have an asoociated identity + assert_err!( + Identity::change_owner_key( + Origin::signed(42), + 10, + TestSignature(10, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + ), + Error::<Test>::IdtyIndexNotFound + ); + + // Payload must be signed by the new key + assert_err!( + Identity::change_owner_key( + Origin::signed(1), + 10, + TestSignature(42, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + ), + Error::<Test>::InvalidNewOwnerKeySig + ); + + // Payload must be prefixed + assert_err!( + Identity::change_owner_key( + Origin::signed(1), + 10, + TestSignature(10, new_key_payload.encode()) + ), + Error::<Test>::InvalidNewOwnerKeySig + ); + + // New owner key should not be used by another identity + assert_err!( + Identity::change_owner_key( + Origin::signed(1), + 2, + TestSignature(2, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + ), + Error::<Test>::OwnerKeyAlreadyUsed + ); + + // Alice can change her owner key + assert_ok!(Identity::change_owner_key( + Origin::signed(1), + 10, + TestSignature(10, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + )); + assert_eq!( + Identity::identity(1), + Some(IdtyVal { + data: (), + next_creatable_identity_on: 0, + old_owner_key: Some((1, 1)), + owner_key: 10, + removable_on: 0, + status: crate::IdtyStatus::Validated, + }) + ); + + run_to_block(2); + + // Alice can't re-change her owner key too early + assert_err!( + Identity::change_owner_key( + Origin::signed(10), + 100, + TestSignature( + 100, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode() + ) + ), + Error::<Test>::OwnerKeyAlreadyRecentlyChanged + ); + }); +} + +#[test] +fn test_idty_revocation() { + new_test_ext(IdentityConfig { + identities: vec![alice()], + }) + .execute_with(|| { let revocation_payload = RevocationPayload { - owner_key: 1, + idty_index: 1u64, genesis_hash: System::block_hash(0), }; + // We need to initialize at least one block before any call + run_to_block(1); + // Payload must be signed by the right identity assert_eq!( Identity::revoke_identity( Origin::signed(1), - revocation_payload.clone(), - TestSignature(42, revocation_payload.encode()) + 1, + 42, + TestSignature(42, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) + ), + Err(Error::<Test>::InvalidRevocationKey.into()) + ); + + // Payload must be prefixed + assert_eq!( + Identity::revoke_identity( + Origin::signed(1), + 1, + 1, + TestSignature(1, revocation_payload.encode()) ), - Err(Error::<Test>::InvalidRevocationProof.into()) + Err(Error::<Test>::InvalidRevocationSig.into()) ); // Anyone can submit a revocation payload assert_ok!(Identity::revoke_identity( Origin::signed(42), - revocation_payload.clone(), - TestSignature(1, revocation_payload.encode()) + 1, + 1, + TestSignature(1, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) )); let events = System::events(); @@ -219,8 +343,9 @@ fn test_idty_revocation() { assert_eq!( Identity::revoke_identity( Origin::signed(1), - revocation_payload.clone(), - TestSignature(1, revocation_payload.encode()) + 1, + 1, + TestSignature(1, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) ), Err(Error::<Test>::IdtyNotFound.into()) ); diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs index d7db3cbe9..0b492553f 100644 --- a/pallets/identity/src/traits.rs +++ b/pallets/identity/src/traits.rs @@ -16,6 +16,8 @@ use crate::*; use frame_support::pallet_prelude::*; +use impl_trait_for_tuples::impl_for_tuples; +use sp_runtime::traits::Saturating; pub trait EnsureIdtyCallAllowed<T: Config> { fn can_create_identity(creator: T::IdtyIndex) -> bool; @@ -40,11 +42,16 @@ pub trait IdtyNameValidator { } pub trait OnIdtyChange<T: Config> { - fn on_idty_change(idty_index: T::IdtyIndex, idty_event: IdtyEvent<T>) -> Weight; + fn on_idty_change(idty_index: T::IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight; } -impl<T: Config> OnIdtyChange<T> for () { - fn on_idty_change(_idty_index: T::IdtyIndex, _idty_event: IdtyEvent<T>) -> Weight { - 0 + +#[impl_for_tuples(5)] +#[allow(clippy::let_and_return)] +impl<T: Config> OnIdtyChange<T> for Tuple { + fn on_idty_change(idty_index: T::IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight { + let mut weight = 0; + for_tuples!( #( weight = weight.saturating_add(Tuple::on_idty_change(idty_index, idty_event)); )* ); + weight } } diff --git a/pallets/identity/src/types.rs b/pallets/identity/src/types.rs index 708b2ed93..824c743f4 100644 --- a/pallets/identity/src/types.rs +++ b/pallets/identity/src/types.rs @@ -27,6 +27,7 @@ pub enum IdtyEvent<T: crate::Config> { Created { creator: T::IdtyIndex }, Confirmed, Validated, + ChangedOwnerKey { new_owner_key: T::AccountId }, Removed { status: IdtyStatus }, } @@ -80,16 +81,25 @@ impl Default for IdtyStatus { #[cfg_attr(feature = "std", derive(Debug, Deserialize, Serialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, TypeInfo)] pub struct IdtyValue<BlockNumber, AccountId, IdtyData> { + pub data: IdtyData, pub next_creatable_identity_on: BlockNumber, + pub old_owner_key: Option<(AccountId, BlockNumber)>, pub owner_key: AccountId, pub removable_on: BlockNumber, pub status: IdtyStatus, - pub data: IdtyData, } -#[derive(Clone, Encode, Decode, PartialEq, Eq, TypeInfo, RuntimeDebug)] -pub struct RevocationPayload<AccountId, Hash> { - pub owner_key: AccountId, - // Avoid replay attack between blockchains +#[derive(Clone, Copy, Encode, RuntimeDebug)] +pub struct NewOwnerKeyPayload<'a, AccountId, IdtyIndex, Hash> { + // Avoid replay attack between networks + pub genesis_hash: &'a Hash, + pub idty_index: IdtyIndex, + pub old_owner_key: &'a AccountId, +} + +#[derive(Clone, Copy, Encode, Decode, PartialEq, Eq, TypeInfo, RuntimeDebug)] +pub struct RevocationPayload<IdtyIndex, Hash> { + // Avoid replay attack between networks pub genesis_hash: Hash, + pub idty_index: IdtyIndex, } diff --git a/runtime/common/src/entities.rs b/runtime/common/src/entities.rs index bcd294197..aa91be374 100644 --- a/runtime/common/src/entities.rs +++ b/runtime/common/src/entities.rs @@ -76,6 +76,29 @@ impl From<IdtyData> for pallet_universal_dividend::FirstEligibleUd { } } +pub struct NewOwnerKeySigner(sp_core::sr25519::Public); + +impl sp_runtime::traits::IdentifyAccount for NewOwnerKeySigner { + type AccountId = crate::AccountId; + fn into_account(self) -> crate::AccountId { + <[u8; 32]>::from(self.0).into() + } +} + +#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo)] +pub struct NewOwnerKeySignature(sp_core::sr25519::Signature); + +impl sp_runtime::traits::Verify for NewOwnerKeySignature { + type Signer = NewOwnerKeySigner; + fn verify<L: sp_runtime::traits::Lazy<[u8]>>(&self, msg: L, signer: &crate::AccountId) -> bool { + use sp_core::crypto::ByteArray as _; + match sp_core::sr25519::Public::from_slice(signer.as_ref()) { + Ok(signer) => self.0.verify(msg, &signer), + Err(()) => false, + } + } +} + #[cfg_attr(feature = "std", derive(Deserialize, Serialize))] #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo)] pub struct SmithsMembershipMetaData<SessionKeysWrapper> { diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs index 92a897143..ca9ccd363 100644 --- a/runtime/common/src/handlers.rs +++ b/runtime/common/src/handlers.rs @@ -21,6 +21,7 @@ use frame_support::instances::{Instance1, Instance2}; use frame_support::pallet_prelude::Weight; use frame_support::traits::Get; use frame_support::Parameter; +use pallet_identity::IdtyEvent; use sp_runtime::traits::IsMember; pub struct OnNewSessionHandler<Runtime>(core::marker::PhantomData<Runtime>); @@ -34,6 +35,36 @@ where } } +pub struct OnIdtyChangeHandler<Runtime>(core::marker::PhantomData<Runtime>); + +impl<T> pallet_identity::traits::OnIdtyChange<T> for OnIdtyChangeHandler<T> +where + T: frame_system::Config<AccountId = AccountId>, + T: pallet_authority_members::Config<MemberId = IdtyIndex>, + T: pallet_identity::Config<IdtyIndex = IdtyIndex>, +{ + fn on_idty_change(idty_index: IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight { + match idty_event { + IdtyEvent::ChangedOwnerKey { new_owner_key } => { + if let Err(e) = pallet_authority_members::Pallet::<T>::change_owner_key( + idty_index, + new_owner_key.clone(), + ) { + log::error!( + "on_idty_change: pallet_authority_members.change_owner_key(): {:?}", + e + ); + } + } + IdtyEvent::Created { .. } + | IdtyEvent::Confirmed + | IdtyEvent::Validated + | IdtyEvent::Removed { .. } => {} + } + 0 + } +} + pub struct OnMembershipEventHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>); type MembershipMetaData = pallet_duniter_wot::MembershipMetaData<AccountId>; diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index 93c7aece4..510c6e237 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -419,6 +419,7 @@ macro_rules! pallets_config { } impl pallet_identity::Config for Runtime { + type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod; type ConfirmPeriod = ConfirmPeriod; type Event = Event; type EnsureIdtyCallAllowed = Wot; @@ -428,7 +429,9 @@ macro_rules! pallets_config { type IdtyNameValidator = IdtyNameValidatorImpl; type IdtyValidationOrigin = EnsureRoot<Self::AccountId>; type IsMember = Membership; - type OnIdtyChange = Wot; + type NewOwnerKeySigner = <NewOwnerKeySignature as sp_runtime::traits::Verify>::Signer; + type NewOwnerKeySignature = NewOwnerKeySignature; + type OnIdtyChange = (common_runtime::handlers::OnIdtyChangeHandler<Runtime>, Wot); type RemoveIdentityConsumers = RemoveIdentityConsumersImpl<Self>; type RevocationSigner = <Signature as sp_runtime::traits::Verify>::Signer; type RevocationSignature = Signature; diff --git a/runtime/g1/src/parameters.rs b/runtime/g1/src/parameters.rs index 9c4db0ceb..44e8ced87 100644 --- a/runtime/g1/src/parameters.rs +++ b/runtime/g1/src/parameters.rs @@ -101,6 +101,7 @@ parameter_types! { // Identity parameter_types! { + pub const ChangeOwnerKeyPeriod: BlockNumber = 6 * MONTHS; pub const ConfirmPeriod: BlockNumber = 14 * DAYS; pub const IdtyCreationPeriod: BlockNumber = MONTHS; pub const ValidationPeriod: BlockNumber = YEARS; diff --git a/runtime/gdev/src/parameters.rs b/runtime/gdev/src/parameters.rs index 274f9ce76..a9fe4dbbc 100644 --- a/runtime/gdev/src/parameters.rs +++ b/runtime/gdev/src/parameters.rs @@ -85,6 +85,15 @@ parameter_types! { pub const SquareMoneyGrowthRate: Perbill = Perbill::from_parts(2_381_440); } +/*******/ +/* WOT */ +/*******/ + +// Identity +frame_support::parameter_types! { + pub const ChangeOwnerKeyPeriod: BlockNumber = 7 * DAYS; +} + /*************/ /* UTILITIES */ /*************/ diff --git a/runtime/gdev/tests/common/mod.rs b/runtime/gdev/tests/common/mod.rs index 34db9d0df..4bcb9d45a 100644 --- a/runtime/gdev/tests/common/mod.rs +++ b/runtime/gdev/tests/common/mod.rs @@ -206,11 +206,12 @@ impl ExtBuilder { index: i as u32 + 1, name: name.clone(), value: IdtyValue { + data: IdtyData::new(), next_creatable_identity_on: Default::default(), owner_key: owner_key.clone(), + old_owner_key: None, removable_on: 0, status: IdtyStatus::Validated, - data: IdtyData::new(), }, }) .collect(), diff --git a/runtime/gtest/src/parameters.rs b/runtime/gtest/src/parameters.rs index fb0c3c801..8193b10c4 100644 --- a/runtime/gtest/src/parameters.rs +++ b/runtime/gtest/src/parameters.rs @@ -103,6 +103,7 @@ parameter_types! { // Identity frame_support::parameter_types! { + pub const ChangeOwnerKeyPeriod: BlockNumber = MONTHS; pub const ConfirmPeriod: BlockNumber = DAYS; pub const IdtyCreationPeriod: BlockNumber = DAYS; } -- GitLab