From 71bc932ef5d305f64b0736f126b7dfcae29fb8bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20Eng=C3=A9libert?= <tuxmain@zettascript.org> Date: Fri, 8 Sep 2023 17:39:51 +0200 Subject: [PATCH] feat: Identity removal reason (nodes/rust/duniter-v2s!179) * Rename associated type Other to OtherReason * Fix benchmarks * feat: Identity removal reason --- pallets/duniter-wot/src/lib.rs | 18 ++++++++++----- pallets/duniter-wot/src/mock.rs | 1 + pallets/duniter-wot/src/tests.rs | 28 +++++++++++++++++++----- pallets/duniter-wot/src/types.rs | 25 +++++++++++++++++++++ pallets/identity/src/benchmarking.rs | 6 ++--- pallets/identity/src/lib.rs | 22 ++++++++++++++----- pallets/identity/src/mock.rs | 1 + pallets/identity/src/tests.rs | 2 ++ pallets/identity/src/types.rs | 8 +++++++ runtime/common/src/pallets_config.rs | 1 + runtime/gdev/tests/integration_tests.rs | 29 +++++++++++++++++++------ 11 files changed, 115 insertions(+), 26 deletions(-) create mode 100644 pallets/duniter-wot/src/types.rs diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs index d897b0f3d..6acb51373 100644 --- a/pallets/duniter-wot/src/lib.rs +++ b/pallets/duniter-wot/src/lib.rs @@ -17,6 +17,8 @@ #![cfg_attr(not(feature = "std"), no_std)] #![allow(clippy::type_complexity)] +mod types; + #[cfg(test)] mod mock; @@ -27,6 +29,7 @@ mod tests; mod benchmarking;*/ pub use pallet::*; +pub use types::*; use frame_support::dispatch::UnfilteredDispatchable; use frame_support::pallet_prelude::*; @@ -57,8 +60,10 @@ pub mod pallet { pub trait Config<I: 'static = ()>: frame_system::Config + pallet_certification::Config<I, IdtyIndex = IdtyIndex> - + pallet_identity::Config<IdtyIndex = IdtyIndex> - + pallet_membership::Config<I, IdtyId = IdtyIndex> + + pallet_identity::Config< + IdtyIndex = IdtyIndex, + IdtyRemovalOtherReason = IdtyRemovalWotReason, + > + pallet_membership::Config<I, IdtyId = IdtyIndex> { #[pallet::constant] type FirstIssuableOn: Get<Self::BlockNumber>; @@ -80,7 +85,7 @@ pub mod pallet { block_number + T::FirstIssuableOn::get(), ); } - pub(super) fn dispath_idty_call(idty_call: pallet_identity::Call<T>) -> bool { + pub(super) fn dispatch_idty_call(idty_call: pallet_identity::Call<T>) -> bool { if !T::IsSubWot::get() { if let Err(e) = idty_call.dispatch_bypass_filter(RawOrigin::Root.into()) { sp_std::if_std! { @@ -288,9 +293,12 @@ where sp_membership::Event::<IdtyIndex, MetaData>::MembershipRenewed(_) => {} sp_membership::Event::<IdtyIndex, MetaData>::MembershipRequested(_) => {} sp_membership::Event::<IdtyIndex, MetaData>::PendingMembershipExpired(idty_index) => { - Self::dispath_idty_call(pallet_identity::Call::remove_identity { + Self::dispatch_idty_call(pallet_identity::Call::remove_identity { idty_index: *idty_index, idty_name: None, + reason: pallet_identity::IdtyRemovalReason::Other( + IdtyRemovalWotReason::MembershipExpired, + ), }); } } @@ -366,7 +374,7 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::OnRemovedCert<IdtyI && pallet_membership::Pallet::<T, I>::is_member(&receiver) { // expire receiver membership - // it gives him a bit of time to get back enough certs + // it gives them a bit of time to get back enough certs if let Err(e) = <pallet_membership::Pallet<T, I>>::force_expire_membership(receiver) { sp_std::if_std! { println!("fail to expire membership: {:?}", e) diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index af88fd49c..d4e95beaa 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -126,6 +126,7 @@ impl pallet_identity::Config for Test { type IdtyData = (); type IdtyNameValidator = IdtyNameValidatorTestImpl; type IdtyIndex = IdtyIndex; + type IdtyRemovalOtherReason = IdtyRemovalWotReason; type NewOwnerKeySigner = UintAuthorityId; type NewOwnerKeySignature = TestSignature; type OnIdtyChange = DuniterWot; diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs index bade3ace7..175f626ae 100644 --- a/pallets/duniter-wot/src/tests.rs +++ b/pallets/duniter-wot/src/tests.rs @@ -14,9 +14,9 @@ // You should have received a copy of the GNU Affero General Public License // along with Duniter-v2S. If not, see <https://www.gnu.org/licenses/>. -use crate::mock::*; use crate::mock::{Identity, System}; use crate::pallet as pallet_duniter_wot; +use crate::{mock::*, IdtyRemovalWotReason}; use codec::Encode; use frame_support::instances::{Instance1, Instance2}; use frame_support::{assert_noop, assert_ok}; @@ -326,7 +326,10 @@ fn test_revoke_idty() { )); System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { idty_index: 2 }, + pallet_identity::Event::IdtyRemoved { + idty_index: 2, + reason: pallet_identity::IdtyRemovalReason::<IdtyRemovalWotReason>::Revoked, + }, )); }); } @@ -351,7 +354,12 @@ fn test_idty_membership_expire() { )); // membership expiry should not trigger identity removal assert!(!System::events().iter().any(|record| record.event - == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved { idty_index: 3 }))); + == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved { + idty_index: 3, + reason: pallet_identity::IdtyRemovalReason::Other( + IdtyRemovalWotReason::MembershipExpired + ) + }))); // it should be moved to pending membership instead assert!(Membership::pending_membership(3).is_some()); @@ -361,7 +369,12 @@ fn test_idty_membership_expire() { pallet_membership::Event::PendingMembershipExpired(3), )); System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { idty_index: 3 }, + pallet_identity::Event::IdtyRemoved { + idty_index: 3, + reason: pallet_identity::IdtyRemovalReason::Other( + IdtyRemovalWotReason::MembershipExpired, + ), + }, )); // Charlie's identity should be removed at block #11 @@ -494,7 +507,12 @@ fn test_certification_expire() { )); // and the identity is removed System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { idty_index: 1 }, + pallet_identity::Event::IdtyRemoved { + idty_index: 1, + reason: pallet_identity::IdtyRemovalReason::Other( + IdtyRemovalWotReason::MembershipExpired, + ), + }, )); }) } diff --git a/pallets/duniter-wot/src/types.rs b/pallets/duniter-wot/src/types.rs new file mode 100644 index 000000000..2a049f86d --- /dev/null +++ b/pallets/duniter-wot/src/types.rs @@ -0,0 +1,25 @@ +// Copyright 2023 Axiom-Team +// +// This file is part of Duniter-v2S. +// +// Duniter-v2S is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, version 3 of the License. +// +// Duniter-v2S is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with Duniter-v2S. If not, see <https://www.gnu.org/licenses/>. + +use codec::{Decode, Encode}; +use frame_support::pallet_prelude::*; +use scale_info::TypeInfo; + +#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)] +pub enum IdtyRemovalWotReason { + MembershipExpired, + Other, +} diff --git a/pallets/identity/src/benchmarking.rs b/pallets/identity/src/benchmarking.rs index ebecf1e33..1cee79c28 100644 --- a/pallets/identity/src/benchmarking.rs +++ b/pallets/identity/src/benchmarking.rs @@ -216,7 +216,7 @@ benchmarks! { let signature = sr25519_sign(0.into(), &caller_public, &message).unwrap().into(); }: _<T::RuntimeOrigin>(account.origin.clone(), account.index.clone().into(), caller.clone(), signature) verify { - assert_has_event::<T>(Event::<T>::IdtyRemoved { idty_index: account.index }.into()); + assert_has_event::<T>(Event::<T>::IdtyRemoved { idty_index: account.index, reason: IdtyRemovalReason::Revoked }.into()); assert!(IdentityIndexOf::<T>::get(&account.key).is_none(), "Identity not revoked"); } @@ -224,13 +224,13 @@ benchmarks! { let new_identity: T::AccountId = account("new_identity", 2, SEED); let account: Account<T> = create_one_identity(new_identity)?; let identities = Pallet::<T>::identities_count(); - }: _<T::RuntimeOrigin>(RawOrigin::Root.into(), account.index.clone(),Some(account.name.clone())) + }: _<T::RuntimeOrigin>(RawOrigin::Root.into(), account.index.clone(), Some(account.name.clone()), IdtyRemovalReason::Manual) verify { assert!( Pallet::<T>::identities_count() == identities - 1, "Identities not removed" ); - assert_has_event::<T>(Event::<T>::IdtyRemoved { idty_index: account.index }.into()); + assert_has_event::<T>(Event::<T>::IdtyRemoved { idty_index: account.index, reason: IdtyRemovalReason::Manual }.into()); assert!(IdentityIndexOf::<T>::get(&account.key).is_none(), "Identity not removed"); } diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index 7ead24e14..d3f7aea4d 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -96,6 +96,8 @@ pub mod pallet { + MaxEncodedLen; /// Handle logic to validate an identity name type IdtyNameValidator: IdtyNameValidator; + /// Additional reasons for identity removal + type IdtyRemovalOtherReason: Clone + Codec + Debug + Eq + TypeInfo; /// On identity confirmed by its owner type OnIdtyChange: OnIdtyChange<Self>; /// Signing key of new owner key payload @@ -248,7 +250,10 @@ pub mod pallet { }, /// An identity has been removed /// [idty_index] - IdtyRemoved { idty_index: T::IdtyIndex }, + IdtyRemoved { + idty_index: T::IdtyIndex, + reason: IdtyRemovalReason<T::IdtyRemovalOtherReason>, + }, } // CALLS // @@ -529,7 +534,7 @@ pub mod pallet { ); // finally if all checks pass, remove identity - Self::do_remove_identity(idty_index); + Self::do_remove_identity(idty_index, IdtyRemovalReason::Revoked); Ok(().into()) } @@ -539,10 +544,11 @@ pub mod pallet { origin: OriginFor<T>, idty_index: T::IdtyIndex, idty_name: Option<IdtyName>, + reason: IdtyRemovalReason<T::IdtyRemovalOtherReason>, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - Self::do_remove_identity(idty_index); + Self::do_remove_identity(idty_index, reason); if let Some(idty_name) = idty_name { <IdentitiesNames<T>>::remove(idty_name); } @@ -662,7 +668,10 @@ pub mod pallet { } /// perform identity removal - pub(super) fn do_remove_identity(idty_index: T::IdtyIndex) -> Weight { + pub(super) fn do_remove_identity( + idty_index: T::IdtyIndex, + reason: IdtyRemovalReason<T::IdtyRemovalOtherReason>, + ) -> Weight { if let Some(idty_val) = Identities::<T>::get(idty_index) { let _ = T::RemoveIdentityConsumers::remove_idty_consumers(idty_index); IdentityIndexOf::<T>::remove(&idty_val.owner_key); @@ -672,7 +681,7 @@ pub mod pallet { if let Some((old_owner_key, _last_change)) = idty_val.old_owner_key { frame_system::Pallet::<T>::dec_sufficients(&old_owner_key); } - Self::deposit_event(Event::IdtyRemoved { idty_index }); + Self::deposit_event(Event::IdtyRemoved { idty_index, reason }); T::OnIdtyChange::on_idty_change( idty_index, &IdtyEvent::Removed { @@ -699,7 +708,8 @@ pub mod pallet { for (idty_index, idty_status) in IdentitiesRemovableOn::<T>::take(block_number) { if let Ok(idty_val) = <Identities<T>>::try_get(idty_index) { if idty_val.removable_on == block_number && idty_val.status == idty_status { - total_weight += Self::do_remove_identity(idty_index) + total_weight += + Self::do_remove_identity(idty_index, IdtyRemovalReason::Expired) } } } diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index 65228040b..9d00c669b 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -108,6 +108,7 @@ impl pallet_identity::Config for Test { type IdtyData = (); type IdtyNameValidator = IdtyNameValidatorTestImpl; type IdtyIndex = u64; + type IdtyRemovalOtherReason = (); type NewOwnerKeySigner = AccountPublic; type NewOwnerKeySignature = Signature; type OnIdtyChange = (); diff --git a/pallets/identity/src/tests.rs b/pallets/identity/src/tests.rs index f04d9d7cb..94016b6be 100644 --- a/pallets/identity/src/tests.rs +++ b/pallets/identity/src/tests.rs @@ -129,6 +129,7 @@ fn test_create_identity_but_not_confirm_it() { System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyRemoved { idty_index: 2, + reason: crate::IdtyRemovalReason::<()>::Expired, })); // We shoud be able to recreate the identity @@ -496,6 +497,7 @@ fn test_idty_revocation() { })); System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyRemoved { idty_index: 1, + reason: crate::IdtyRemovalReason::<()>::Revoked, })); run_to_block(2); diff --git a/pallets/identity/src/types.rs b/pallets/identity/src/types.rs index 1d633e293..e7c98f307 100644 --- a/pallets/identity/src/types.rs +++ b/pallets/identity/src/types.rs @@ -37,6 +37,14 @@ pub enum IdtyEvent<T: crate::Config> { Removed { status: IdtyStatus }, } +#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)] +pub enum IdtyRemovalReason<OtherReason: TypeInfo + Decode + Encode + Eq + Clone> { + Expired, + Manual, + Other(OtherReason), + Revoked, +} + /// name of the identity, ascii encoded #[derive(Encode, Decode, Default, Clone, PartialEq, Eq, PartialOrd, Ord, RuntimeDebug)] pub struct IdtyName(pub Vec<u8>); diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index c243beb1d..36c0a294f 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -443,6 +443,7 @@ macro_rules! pallets_config { type IdtyData = IdtyData; type IdtyIndex = IdtyIndex; type IdtyNameValidator = IdtyNameValidatorImpl; + type IdtyRemovalOtherReason = pallet_duniter_wot::IdtyRemovalWotReason; type NewOwnerKeySigner = <NewOwnerKeySignature as sp_runtime::traits::Verify>::Signer; type NewOwnerKeySignature = NewOwnerKeySignature; type OnIdtyChange = (common_runtime::handlers::OnIdtyChangeHandler<Runtime>, Wot); diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs index 0bb497e1e..f18dbca93 100644 --- a/runtime/gdev/tests/integration_tests.rs +++ b/runtime/gdev/tests/integration_tests.rs @@ -90,7 +90,8 @@ fn test_remove_identity() { assert_ok!(Identity::remove_identity( frame_system::RawOrigin::Root.into(), 4, - None + None, + pallet_identity::IdtyRemovalReason::Manual )); System::assert_has_event(RuntimeEvent::Membership( @@ -100,7 +101,10 @@ fn test_remove_identity() { account: AccountKeyring::Dave.to_account_id(), })); System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { idty_index: 4 }, + pallet_identity::Event::IdtyRemoved { + idty_index: 4, + reason: pallet_identity::IdtyRemovalReason::Manual, + }, )); }); } @@ -167,7 +171,10 @@ fn test_membership_expiry() { )); // membership expiry should not trigger identity removal assert!(!System::events().iter().any(|record| record.event - == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved { idty_index: 1 }))); + == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved { + idty_index: 1, + reason: pallet_identity::IdtyRemovalReason::Expired + }))); }); } @@ -215,7 +222,8 @@ fn test_remove_identity_after_one_ud() { assert_ok!(Identity::remove_identity( frame_system::RawOrigin::Root.into(), 4, - None + None, + pallet_identity::IdtyRemovalReason::Manual )); // Verify events @@ -238,7 +246,10 @@ fn test_remove_identity_after_one_ud() { }, )); System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { idty_index: 4 }, + pallet_identity::Event::IdtyRemoved { + idty_index: 4, + reason: pallet_identity::IdtyRemovalReason::Manual, + }, )); // Verify state @@ -347,7 +358,8 @@ fn test_remove_smith_identity() { assert_ok!(Identity::remove_identity( frame_system::RawOrigin::Root.into(), 3, - None + None, + pallet_identity::IdtyRemovalReason::Manual )); // Verify events System::assert_has_event(RuntimeEvent::SmithMembership( @@ -360,7 +372,10 @@ fn test_remove_smith_identity() { pallet_membership::Event::MembershipRevoked(3), )); System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { idty_index: 3 }, + pallet_identity::Event::IdtyRemoved { + idty_index: 3, + reason: pallet_identity::IdtyRemovalReason::Manual, + }, )); }); } -- GitLab