diff --git a/docs/api/runtime-events.md b/docs/api/runtime-events.md index 3ad31145660822fa4e259986ebaa5dddfaf27c2d..cd058e2b44ecf4afb7ca98486c4ad0aefeb5649d 100644 --- a/docs/api/runtime-events.md +++ b/docs/api/runtime-events.md @@ -1077,7 +1077,7 @@ members_count: BalanceOf<T> <li> <details> <summary> -<code>UdsAutoPaidAtRemoval(count, total, who)</code> - 2</summary> +<code>UdsAutoPaid(count, total, who)</code> - 2</summary> DUs were automatically transferred as part of a member removal. ```rust diff --git a/notes.txt b/notes.txt index 7a8a225c85a2d057b0e39b7ac424151a998abe27..d54aa612ba7b20db5610de06124627adc79edb9c 100644 --- a/notes.txt +++ b/notes.txt @@ -2,14 +2,13 @@ - à prévoir trouver un meilleur nom pour "next_scheduled" (anciennement removable_on) qui correspond à la prochaine action programmée exécuter les benchmarks ajouter quelques benchmarks vérifier les scénarios pour revoke_membership - +ajouter un live test pour vérifier la cohérence de IdtyStatus::(Not)Member et la présence d'une membership optionnellement diff --git a/pallets/distance/src/mock.rs b/pallets/distance/src/mock.rs index 5c9cf7365f38f7a22bab6784eb79e1dcfe0d12ee..988c9b43ff3893da774cdc707856fd0953d59e2a 100644 --- a/pallets/distance/src/mock.rs +++ b/pallets/distance/src/mock.rs @@ -243,7 +243,6 @@ impl pallet_identity::Config for Test { type Signer = UintAuthorityId; type Signature = TestSignature; type OnIdtyChange = (); - type RemoveIdentityConsumers = (); type RuntimeEvent = RuntimeEvent; type WeightInfo = (); #[cfg(feature = "runtime-benchmarks")] diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index 06d26386841923b4c1369ffb2ee691c9caf5afa1..9420b482f4f48a76047750cfdd8d289f1677f83b 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -136,7 +136,6 @@ impl pallet_identity::Config for Test { type Signer = UintAuthorityId; type Signature = TestSignature; type OnIdtyChange = (DuniterWot, SmithSubWot); - type RemoveIdentityConsumers = (); type RuntimeEvent = RuntimeEvent; type WeightInfo = (); #[cfg(feature = "runtime-benchmarks")] diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index d57cbf968a192566d9bddf8d8417477cbe33b7fb..faead509636312b0e9e34d61ca6d5fbcf51c4d6a 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -121,9 +121,6 @@ pub mod pallet { type Signer: IdentifyAccount<AccountId = Self::AccountId>; /// Signature of a payload type Signature: Parameter + Verify<Signer = Self::Signer>; - /// 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>; /// Because this pallet emits events, it depends on the runtime's definition of an event. type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>; /// Type representing the weight of this pallet @@ -677,9 +674,11 @@ pub mod pallet { } /// perform identity removal + // (kind of garbage collector) + // this should not be called while the identity is still member + // otherwise there will still be a membership in storage, but no more identity pub fn do_remove_identity(idty_index: T::IdtyIndex, reason: RemovalReason) -> Weight { if let Some(idty_value) = Identities::<T>::get(idty_index) { - let _ = T::RemoveIdentityConsumers::remove_idty_consumers(idty_index); // this line allows the owner key to be used after that IdentityIndexOf::<T>::remove(&idty_value.owner_key); // Identity should be removed after the consumers of the identity diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index 0ecf89813d7be972bb02057e7e9bb7ffec20aaa4..9421a70f03547f9b1f426658289cbf9f1b870025 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -116,7 +116,6 @@ impl pallet_identity::Config for Test { type Signer = AccountPublic; type Signature = Signature; type OnIdtyChange = (); - type RemoveIdentityConsumers = (); type RuntimeEvent = RuntimeEvent; type WeightInfo = (); #[cfg(feature = "runtime-benchmarks")] diff --git a/pallets/identity/src/tests.rs b/pallets/identity/src/tests.rs index 20a303d42ac8985f7c12b6b2e89dc6958b2717e9..7d8c3abc618523f278b2cddcb40e5483b1beeffa 100644 --- a/pallets/identity/src/tests.rs +++ b/pallets/identity/src/tests.rs @@ -108,8 +108,8 @@ fn test_identity_index() { }) .execute_with(|| { assert_eq!(Identity::identities_count(), 2); - // assert_eq!(NextIdtyIndex, 3); // TODO see if we can debug that - // TODO create identity and check it was incremented + // assert_eq!(NextIdtyIndex, 3); // TODO check how to test that + // ... create identity and check it was incremented }); } diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs index 0ecf15a0c1b5375226fe6f3590a928ce31c2be5a..a152bd39c13c00bb67ad08cece0bb4d9d49c1c11 100644 --- a/pallets/identity/src/traits.rs +++ b/pallets/identity/src/traits.rs @@ -51,15 +51,6 @@ impl<T: Config> OnIdtyChange<T> for Tuple { } } -pub trait RemoveIdentityConsumers<IndtyIndex> { - fn remove_idty_consumers(idty_index: IndtyIndex) -> Weight; -} -impl<IndtyIndex> RemoveIdentityConsumers<IndtyIndex> for () { - fn remove_idty_consumers(_: IndtyIndex) -> Weight { - Weight::zero() - } -} - /// trait used to link an account to an identity pub trait LinkIdty<AccountId, IdtyIndex> { fn link_identity(account_id: AccountId, idty_index: IdtyIndex); diff --git a/pallets/quota/src/mock.rs b/pallets/quota/src/mock.rs index e6fd1a7021527973126a6634182f7981b40cddcf..d023eebeefd5907e62481cf589bab26a531f8cc4 100644 --- a/pallets/quota/src/mock.rs +++ b/pallets/quota/src/mock.rs @@ -162,7 +162,6 @@ impl pallet_identity::Config for Test { type Signer = AccountPublic; type Signature = Signature; type OnIdtyChange = (); - type RemoveIdentityConsumers = (); type RuntimeEvent = RuntimeEvent; type WeightInfo = (); #[cfg(feature = "runtime-benchmarks")] diff --git a/pallets/universal-dividend/src/benchmarking.rs b/pallets/universal-dividend/src/benchmarking.rs index 99a3929377c4397289c291d295f0e52e75076e77..5786d2ee0930ddf30ce14f5766aa412c77ecce82 100644 --- a/pallets/universal-dividend/src/benchmarking.rs +++ b/pallets/universal-dividend/src/benchmarking.rs @@ -111,7 +111,7 @@ benchmarks! { }: {Pallet::<T>::on_removed_member(CurrentUdIndex::<T>::get() - i as u16, &caller);} verify { if i != 0 { - assert_has_event::<T>(Event::<T>::UdsAutoPaidAtRemoval {count: i as u16, total: uds_total, who: caller}.into()); + assert_has_event::<T>(Event::<T>::UdsAutoPaid {count: i as u16, total: uds_total, who: caller}.into()); } } diff --git a/pallets/universal-dividend/src/lib.rs b/pallets/universal-dividend/src/lib.rs index 09ba8511dc786cd3f8e534fbec2dc622c144dce1..b41e4b245bc7bc0e3e6ff8c4708ea776064a92de 100644 --- a/pallets/universal-dividend/src/lib.rs +++ b/pallets/universal-dividend/src/lib.rs @@ -236,7 +236,7 @@ pub mod pallet { members_count: BalanceOf<T>, }, /// DUs were automatically transferred as part of a member removal. - UdsAutoPaidAtRemoval { + UdsAutoPaid { count: UdIndex, total: BalanceOf<T>, who: T::AccountId, @@ -442,7 +442,7 @@ pub mod pallet { PastReevals::<T>::get().into_iter(), ); T::Currency::deposit_creating(who, uds_total); - Self::deposit_event(Event::UdsAutoPaidAtRemoval { + Self::deposit_event(Event::UdsAutoPaid { count: uds_count, total: uds_total, who: who.clone(), diff --git a/primitives/membership/src/lib.rs b/primitives/membership/src/lib.rs index cf80b19ccc565b6e740f0eaa70be40ec634c9b8b..897bf64ffc81003caef3f02a60aff9251f2b7945 100644 --- a/primitives/membership/src/lib.rs +++ b/primitives/membership/src/lib.rs @@ -27,7 +27,7 @@ use scale_info::TypeInfo; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; -// TODO remove these event primitives and migrate them to handlers +/// membership events pub enum Event<IdtyId> { /// A membership was acquired. MembershipAdded(IdtyId), @@ -37,7 +37,6 @@ pub enum Event<IdtyId> { MembershipRenewed(IdtyId), } -// TODO internalize membership primitives in membership pallet #[cfg_attr(feature = "std", derive(Deserialize, Serialize))] #[derive(Encode, Decode, Default, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo)] pub struct MembershipData<BlockNumber: Decode + Encode + TypeInfo> { diff --git a/resources/metadata.scale b/resources/metadata.scale index ee96bcfccb0837ee5d73cc56258480ae34773aa4..60cf370fe64aecd0dc9bbdd157c56e90b5653228 100644 Binary files a/resources/metadata.scale and b/resources/metadata.scale differ diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs index ae6132a757a83d2eb2c29d59cbe89ec97aae7873..8cb679c6680d6a74ac760d6a4556dc6744fbc4df 100644 --- a/runtime/common/src/handlers.rs +++ b/runtime/common/src/handlers.rs @@ -20,7 +20,6 @@ use frame_support::dispatch::UnfilteredDispatchable; use frame_support::instances::{Instance1, Instance2}; use frame_support::pallet_prelude::Weight; use frame_support::Parameter; -use sp_runtime::traits::IsMember; // new session handler pub struct OnNewSessionHandler<Runtime>(core::marker::PhantomData<Runtime>); @@ -115,6 +114,7 @@ impl< // authority member removal handler // TODO refac smith wot +// or document the link between smith membership and authority member pub struct OnRemovedAuthorityMemberHandler<Runtime>(core::marker::PhantomData<Runtime>); impl<Runtime> pallet_authority_members::traits::OnRemovedMember<IdtyIndex> for OnRemovedAuthorityMemberHandler<Runtime> @@ -122,7 +122,6 @@ where Runtime: frame_system::Config + pallet_membership::Config<Instance2, IdtyId = IdtyIndex>, { fn on_removed_member(idty_index: IdtyIndex) { - // TODO investigate why we should remove smith membership when removing authority member pallet_membership::Pallet::<Runtime, Instance2>::do_remove_membership( idty_index, pallet_membership::MembershipRemovalReason::System, @@ -130,35 +129,6 @@ where } } -// identity removal handler -// TODO check if it is really useful -pub struct RemoveIdentityConsumersImpl<Runtime>(core::marker::PhantomData<Runtime>); -impl<Runtime> pallet_identity::traits::RemoveIdentityConsumers<IdtyIndex> - for RemoveIdentityConsumersImpl<Runtime> -where - Runtime: pallet_identity::Config<IdtyIndex = IdtyIndex> - + pallet_authority_members::Config<MemberId = IdtyIndex> - + pallet_membership::Config<Instance1, IdtyId = IdtyIndex> - + pallet_membership::Config<Instance2, IdtyId = IdtyIndex>, -{ - fn remove_idty_consumers(idty_index: IdtyIndex) -> Weight { - // Remove smith member - if pallet_membership::Pallet::<Runtime, Instance2>::is_member(&idty_index) { - pallet_membership::Pallet::<Runtime, Instance2>::do_remove_membership( - idty_index, - pallet_membership::MembershipRemovalReason::System, - ); - } - // Remove "classic" member - pallet_membership::Pallet::<Runtime, Instance1>::do_remove_membership( - idty_index, - pallet_membership::MembershipRemovalReason::System, - ); - - Weight::zero() - } -} - // spend treasury handler pub struct TreasurySpendFunds<Runtime>(core::marker::PhantomData<Runtime>); impl<Runtime> pallet_treasury::SpendFunds<Runtime> for TreasurySpendFunds<Runtime> diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index 74338942a06d44601c7af66b93b4b7358a86abfc..471c97672353b4b91bb648a14089a582a7c8e9fa 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -480,7 +480,6 @@ macro_rules! pallets_config { type Signer = <Signature as sp_runtime::traits::Verify>::Signer; type Signature = Signature; type OnIdtyChange = (Wot, SmithSubWot, Quota, Account); - type RemoveIdentityConsumers = RemoveIdentityConsumersImpl<Self>; type RuntimeEvent = RuntimeEvent; type WeightInfo = common_runtime::weights::pallet_identity::WeightInfo<Runtime>; #[cfg(feature = "runtime-benchmarks")] diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs index 71506530e8c18c206c201d33dff776846444a23a..fa700844761e49eaa732756378ea805f4f2d9c0e 100644 --- a/runtime/gdev/tests/integration_tests.rs +++ b/runtime/gdev/tests/integration_tests.rs @@ -290,12 +290,13 @@ fn test_remove_identity() { run_to_block(2); // remove the identity Identity::do_remove_identity(4, pallet_identity::RemovalReason::Root); - System::assert_has_event(RuntimeEvent::Membership( - pallet_membership::Event::MembershipRemoved { - member: 4, - reason: MembershipRemovalReason::System, - }, - )); + // // membership removal is no more automatic + // System::assert_has_event(RuntimeEvent::Membership( + // pallet_membership::Event::MembershipRemoved { + // member: 4, + // reason: MembershipRemovalReason::System, + // }, + // )); System::assert_has_event(RuntimeEvent::Identity( pallet_identity::Event::IdtyRemoved { idty_index: 4, @@ -475,9 +476,9 @@ fn test_membership_renewal() { }); } -// test that UD are auto claimed when identity is removed +// test that UD are auto claimed when identity is revoked #[test] -fn test_remove_identity_after_one_ud() { +fn test_revoke_identity_after_one_ud() { ExtBuilder::new(1, 3, 4).build().execute_with(|| { //println!("UdCreationPeriod={}", <Runtime as pallet_universal_dividend::Config>::UdCreationPeriod::get()); run_to_block( @@ -499,13 +500,13 @@ fn test_remove_identity_after_one_ud() { / <Runtime as pallet_babe::Config>::ExpectedBlockTime::get() + 1) as u32, ); - // remove identity - Identity::do_remove_identity(4, pallet_identity::RemovalReason::Root); + // revoke identity + Identity::do_revoke_identity(4, pallet_identity::RevocationReason::Root); // Verify events // universal dividend was automatically paid to dave System::assert_has_event(RuntimeEvent::UniversalDividend( - pallet_universal_dividend::Event::UdsAutoPaidAtRemoval { + pallet_universal_dividend::Event::UdsAutoPaid { count: 1, total: 1_000, who: AccountKeyring::Dave.to_account_id(), @@ -520,18 +521,17 @@ fn test_remove_identity_after_one_ud() { System::assert_has_event(RuntimeEvent::Membership( pallet_membership::Event::MembershipRemoved { member: 4, - reason: MembershipRemovalReason::System, + reason: MembershipRemovalReason::Revoked, }, )); System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { + pallet_identity::Event::IdtyRevoked { idty_index: 4, - reason: pallet_identity::RemovalReason::Root, + reason: pallet_identity::RevocationReason::Root, }, )); - // thanks to the new UD, Dave has existential deposit and is not killed - assert!(Identity::identity(4).is_none()); + assert!(Identity::identity(4).is_some()); // identity still exists, but its status is revoked assert_eq!( Balances::free_balance(AccountKeyring::Dave.to_account_id()), 1_000 @@ -564,7 +564,7 @@ fn test_ud_claimed_membership_on_and_off() { // alice identity expires Membership::do_remove_membership(1, MembershipRemovalReason::System); System::assert_has_event(RuntimeEvent::UniversalDividend( - pallet_universal_dividend::Event::UdsAutoPaidAtRemoval { + pallet_universal_dividend::Event::UdsAutoPaid { count: 1, total: 1_000, who: AccountKeyring::Alice.to_account_id(), @@ -639,18 +639,18 @@ fn test_ud_claimed_membership_on_and_off() { }); } -/// test when root removes and identity, all consumers should be deleted +/// test when root revokes and identity, all membership should be deleted #[test] -fn test_remove_smith_identity() { +fn test_revoke_smith_identity() { ExtBuilder::new(1, 3, 4).build().execute_with(|| { run_to_block(2); - Identity::do_remove_identity(3, pallet_identity::RemovalReason::Root); + Identity::do_revoke_identity(3, pallet_identity::RevocationReason::Root); // Verify events System::assert_has_event(RuntimeEvent::SmithMembership( pallet_membership::Event::MembershipRemoved { member: 3, - reason: MembershipRemovalReason::System, + reason: MembershipRemovalReason::Revoked, }, )); System::assert_has_event(RuntimeEvent::AuthorityMembers( @@ -659,13 +659,13 @@ fn test_remove_smith_identity() { System::assert_has_event(RuntimeEvent::Membership( pallet_membership::Event::MembershipRemoved { member: 3, - reason: MembershipRemovalReason::System, + reason: MembershipRemovalReason::Revoked, }, )); System::assert_has_event(RuntimeEvent::Identity( - pallet_identity::Event::IdtyRemoved { + pallet_identity::Event::IdtyRevoked { idty_index: 3, - reason: pallet_identity::RemovalReason::Root, + reason: pallet_identity::RevocationReason::Root, }, )); }); diff --git a/runtime/gtest/src/lib.rs b/runtime/gtest/src/lib.rs index 78311d5212c8a0d53b50f54013fa0e3010a18552..88fbd282f83728cf10ee03e282715b7ec34ebf67 100644 --- a/runtime/gtest/src/lib.rs +++ b/runtime/gtest/src/lib.rs @@ -171,16 +171,10 @@ mod benches { pub struct BaseCallFilter; // implement filter +// session pallet calls are filtered in order to use authority member instead impl Contains<RuntimeCall> for BaseCallFilter { fn contains(call: &RuntimeCall) -> bool { - !matches!( - call, - // in main web of trust, membership request and revoke are handeled through identity pallet - // the user can not call them directly - // TODO allow revoke membership - RuntimeCall::Membership(pallet_membership::Call::revoke_membership { .. }) - | RuntimeCall::Session(_) - ) + !matches!(call, RuntimeCall::Session(_)) } }