From ec620b8a9d490f9d8c0b124e4a9920e3c2439fe7 Mon Sep 17 00:00:00 2001 From: bgallois <benjamin@gallois.cc> Date: Thu, 7 Mar 2024 13:30:07 +0100 Subject: [PATCH] simplify IdtyChange handlers --- pallets/distance/src/mock.rs | 3 +- pallets/duniter-wot/src/lib.rs | 70 ++++++++++++---------------- pallets/duniter-wot/src/mock.rs | 3 +- pallets/identity/src/benchmarking.rs | 7 +-- pallets/identity/src/lib.rs | 28 +++-------- pallets/identity/src/mock.rs | 3 +- pallets/identity/src/traits.rs | 31 ++++++++++-- pallets/quota/src/lib.rs | 47 +++++++++++-------- pallets/quota/src/mock.rs | 3 +- runtime/common/src/pallets_config.rs | 3 +- 10 files changed, 101 insertions(+), 97 deletions(-) diff --git a/pallets/distance/src/mock.rs b/pallets/distance/src/mock.rs index b880b29cf..86337cce6 100644 --- a/pallets/distance/src/mock.rs +++ b/pallets/distance/src/mock.rs @@ -236,7 +236,8 @@ impl pallet_identity::Config for Test { type IdtyData = (); type IdtyIndex = u32; type IdtyNameValidator = IdtyNameValidatorTestImpl; - type OnIdtyChange = (); + type OnNewIdty = (); + type OnRemoveIdty = (); type RuntimeEvent = RuntimeEvent; type Signature = TestSignature; type Signer = UintAuthorityId; diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs index b84102845..a3076f124 100644 --- a/pallets/duniter-wot/src/lib.rs +++ b/pallets/duniter-wot/src/lib.rs @@ -32,7 +32,7 @@ pub use pallet::*; use frame_support::pallet_prelude::*; use pallet_certification::traits::SetNextIssuableOn; -use pallet_identity::{IdtyEvent, IdtyStatus}; +use pallet_identity::IdtyStatus; use pallet_membership::MembershipRemovalReason; type IdtyIndex = u32; @@ -221,46 +221,36 @@ where } // implement identity event handler -impl<T: Config> pallet_identity::traits::OnIdtyChange<T> for Pallet<T> { - fn on_idty_change(idty_index: IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight { +impl<T: Config> pallet_identity::traits::OnNewIdty<T> for Pallet<T> { + fn on_created(idty_index: &IdtyIndex, creator: &IdtyIndex) -> Weight { let mut weight = Weight::zero(); - match idty_event { - // identity just has been created, a cert must be added - IdtyEvent::Created { creator, .. } => { - weight = - weight.saturating_add(<pallet_certification::Pallet<T>>::do_add_cert_checked( - *creator, idty_index, true, - )); - } - // we could split this event in removed / revoked: - // if identity is revoked keep it - // if identity is removed also remove certs - IdtyEvent::Removed { status } => { - // try remove membership in any case - weight = - weight.saturating_add(<pallet_membership::Pallet<T>>::do_remove_membership( - idty_index, - MembershipRemovalReason::Revoked, - )); - - // only remove certs if identity is unvalidated - match status { - IdtyStatus::Unconfirmed | IdtyStatus::Unvalidated => { - weight = weight.saturating_add( - <pallet_certification::Pallet<T>>::do_remove_all_certs_received_by( - idty_index, - ), - ); - } - IdtyStatus::Revoked => {} - IdtyStatus::Member | IdtyStatus::NotMember => { - sp_std::if_std! { - println!("removed non-revoked identity: {:?}", idty_index); - } - } - } - } - } + // identity just has been created, a cert must be added + weight = weight.saturating_add(<pallet_certification::Pallet<T>>::do_add_cert_checked( + *creator, + *idty_index, + true, + )); + weight + } +} + +impl<T: Config> pallet_identity::traits::OnRemoveIdty<T> for Pallet<T> { + // Remove membership and certificates. + fn on_removed(idty_index: &IdtyIndex) -> Weight { + let mut weight = Self::on_revoked(idty_index); + weight = weight.saturating_add( + <pallet_certification::Pallet<T>>::do_remove_all_certs_received_by(*idty_index), + ); + weight + } + + // Remove membership only. + fn on_revoked(idty_index: &IdtyIndex) -> Weight { + let mut weight = Weight::zero(); + weight = weight.saturating_add(<pallet_membership::Pallet<T>>::do_remove_membership( + *idty_index, + MembershipRemovalReason::Revoked, + )); weight } } diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index 29254167f..0a442ac5d 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -124,7 +124,8 @@ impl pallet_identity::Config for Test { type IdtyData = (); type IdtyIndex = IdtyIndex; type IdtyNameValidator = IdtyNameValidatorTestImpl; - type OnIdtyChange = DuniterWot; + type OnNewIdty = DuniterWot; + type OnRemoveIdty = DuniterWot; type RuntimeEvent = RuntimeEvent; type Signature = TestSignature; type Signer = UintAuthorityId; diff --git a/pallets/identity/src/benchmarking.rs b/pallets/identity/src/benchmarking.rs index fc311a0e9..f693a5de7 100644 --- a/pallets/identity/src/benchmarking.rs +++ b/pallets/identity/src/benchmarking.rs @@ -447,12 +447,7 @@ mod benchmarks { #[block] { - T::OnIdtyChange::on_idty_change( - idty_index, - &IdtyEvent::Removed { - status: IdtyStatus::Member, - }, - ); + T::OnRemoveIdty::on_removed(&idty_index); } } diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index 6e5c07c58..3ff6fde96 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -115,8 +115,10 @@ pub mod pallet { type AccountLinker: LinkIdty<Self::AccountId, Self::IdtyIndex>; /// Handle logic to validate an identity name type IdtyNameValidator: IdtyNameValidator; - /// On identity confirmed by its owner - type OnIdtyChange: OnIdtyChange<Self>; + /// On identity created. + type OnNewIdty: OnNewIdty<Self>; + /// On identity removed. + type OnRemoveIdty: OnRemoveIdty<Self>; /// Signing key of a payload type Signer: IdentifyAccount<AccountId = Self::AccountId>; /// Signature of a payload @@ -325,13 +327,7 @@ pub mod pallet { owner_key: owner_key.clone(), }); T::AccountLinker::link_identity(&owner_key, idty_index)?; - T::OnIdtyChange::on_idty_change( - idty_index, - &IdtyEvent::Created { - creator: creator_index, - owner_key, - }, - ); + T::OnNewIdty::on_created(&idty_index, &creator_index); Ok(().into()) } @@ -686,12 +682,7 @@ pub mod pallet { frame_system::Pallet::<T>::dec_sufficients(&old_owner_key); } Self::deposit_event(Event::IdtyRemoved { idty_index, reason }); - let weight = T::OnIdtyChange::on_idty_change( - idty_index, - &IdtyEvent::Removed { - status: idty_value.status, - }, - ); + let weight = T::OnRemoveIdty::on_removed(&idty_index); return weight.saturating_add( T::WeightInfo::do_remove_identity() .saturating_sub(T::WeightInfo::do_remove_identity_handler()), @@ -711,12 +702,7 @@ pub mod pallet { ); Self::deposit_event(Event::IdtyRevoked { idty_index, reason }); - T::OnIdtyChange::on_idty_change( - idty_index, - &IdtyEvent::Removed { - status: IdtyStatus::Revoked, - }, - ); + T::OnRemoveIdty::on_revoked(&idty_index); return T::WeightInfo::do_revoke_identity(); } T::WeightInfo::do_revoke_identity_noop() diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index 894154414..8a887b008 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -109,7 +109,8 @@ impl pallet_identity::Config for Test { type IdtyData = (); type IdtyIndex = u64; type IdtyNameValidator = IdtyNameValidatorTestImpl; - type OnIdtyChange = (); + type OnNewIdty = (); + type OnRemoveIdty = (); type RuntimeEvent = RuntimeEvent; type Signature = Signature; type Signer = AccountPublic; diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs index ced84de96..ed8abc16a 100644 --- a/pallets/identity/src/traits.rs +++ b/pallets/identity/src/traits.rs @@ -35,16 +35,37 @@ pub trait IdtyNameValidator { fn validate(idty_name: &IdtyName) -> bool; } -pub trait OnIdtyChange<T: Config> { - fn on_idty_change(idty_index: T::IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight; +pub trait OnNewIdty<T: Config> { + fn on_created(idty_index: &T::IdtyIndex, creator: &T::IdtyIndex) -> Weight; +} + +pub trait OnRemoveIdty<T: Config> { + fn on_removed(idty_index: &T::IdtyIndex) -> Weight; + fn on_revoked(idty_index: &T::IdtyIndex) -> Weight; } #[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 { +impl<T: Config> OnNewIdty<T> for Tuple { + fn on_created(idty_index: &T::IdtyIndex, creator: &T::IdtyIndex) -> Weight { + let mut weight = Weight::zero(); + for_tuples!( #( weight = weight.saturating_add(Tuple::on_created(idty_index, creator)); )* ); + weight + } +} + +#[impl_for_tuples(5)] +#[allow(clippy::let_and_return)] +impl<T: Config> OnRemoveIdty<T> for Tuple { + fn on_removed(idty_index: &T::IdtyIndex) -> Weight { + let mut weight = Weight::zero(); + for_tuples!( #( weight = weight.saturating_add(Tuple::on_removed(idty_index)); )* ); + weight + } + + fn on_revoked(idty_index: &T::IdtyIndex) -> Weight { let mut weight = Weight::zero(); - for_tuples!( #( weight = weight.saturating_add(Tuple::on_idty_change(idty_index, idty_event)); )* ); + for_tuples!( #( weight = weight.saturating_add(Tuple::on_revoked(idty_index)); )* ); weight } } diff --git a/pallets/quota/src/lib.rs b/pallets/quota/src/lib.rs index 83a28c67d..398f67aad 100644 --- a/pallets/quota/src/lib.rs +++ b/pallets/quota/src/lib.rs @@ -33,7 +33,6 @@ use frame_support::pallet_prelude::*; use frame_support::traits::{Currency, ExistenceRequirement}; use frame_system::pallet_prelude::*; pub use pallet::*; -use pallet_identity::IdtyEvent; use sp_runtime::traits::Zero; use sp_std::fmt::Debug; use sp_std::vec::Vec; @@ -339,30 +338,38 @@ fn is_eligible_for_refund<T: pallet_identity::Config>(_identity: IdtyId<T>) -> b } // implement identity event handler -impl<T: Config> pallet_identity::traits::OnIdtyChange<T> for Pallet<T> { - fn on_idty_change(idty_id: IdtyId<T>, idty_event: &IdtyEvent<T>) -> Weight { +impl<T: Config> pallet_identity::traits::OnNewIdty<T> for Pallet<T> { + fn on_created(idty_index: &IdtyId<T>, _creator: &T::IdtyIndex) -> Weight { let mut weight = Weight::zero(); let mut add_db_reads_writes = |reads, writes| { weight = weight.saturating_add(T::DbWeight::get().reads_writes(reads, writes)); }; - match idty_event { - // initialize quota on identity creation - IdtyEvent::Created { .. } => { - IdtyQuota::<T>::insert( - idty_id, - Quota { - last_use: frame_system::pallet::Pallet::<T>::block_number(), - amount: BalanceOf::<T>::zero(), - }, - ); - add_db_reads_writes(0, 1); - } - IdtyEvent::Removed { .. } => { - IdtyQuota::<T>::remove(idty_id); - add_db_reads_writes(1, 1); - } - } + IdtyQuota::<T>::insert( + idty_index, + Quota { + last_use: frame_system::pallet::Pallet::<T>::block_number(), + amount: BalanceOf::<T>::zero(), + }, + ); + add_db_reads_writes(0, 1); weight } } + +impl<T: Config> pallet_identity::traits::OnRemoveIdty<T> for Pallet<T> { + fn on_removed(idty_id: &IdtyId<T>) -> Weight { + let mut weight = Weight::zero(); + let mut add_db_reads_writes = |reads, writes| { + weight = weight.saturating_add(T::DbWeight::get().reads_writes(reads, writes)); + }; + + IdtyQuota::<T>::remove(idty_id); + add_db_reads_writes(1, 1); + weight + } + + fn on_revoked(idty_id: &IdtyId<T>) -> Weight { + Self::on_removed(idty_id) + } +} diff --git a/pallets/quota/src/mock.rs b/pallets/quota/src/mock.rs index a3bb50f43..b5fa5311f 100644 --- a/pallets/quota/src/mock.rs +++ b/pallets/quota/src/mock.rs @@ -152,7 +152,8 @@ impl pallet_identity::Config for Test { type IdtyData = (); type IdtyIndex = u64; type IdtyNameValidator = IdtyNameValidatorTestImpl; - type OnIdtyChange = (); + type OnNewIdty = (); + type OnRemoveIdty = (); type RuntimeEvent = RuntimeEvent; type Signature = Signature; type Signer = AccountPublic; diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index c713b5bfb..4ab357080 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -481,7 +481,8 @@ type RuntimeFreezeReason = (); type IdtyNameValidator = IdtyNameValidatorImpl; type Signer = <Signature as sp_runtime::traits::Verify>::Signer; type Signature = Signature; - type OnIdtyChange = (Wot, Quota); + type OnNewIdty = (Wot, Quota); + type OnRemoveIdty = (Wot, Quota); type RuntimeEvent = RuntimeEvent; type WeightInfo = common_runtime::weights::pallet_identity::WeightInfo<Runtime>; } -- GitLab