From 5159922cc10646d909882fc14042ebdf2ea07c44 Mon Sep 17 00:00:00 2001 From: mildred <robot-duniter@mildred.fr> Date: Wed, 7 Sep 2022 17:17:14 +0200 Subject: [PATCH] Remove RevocationPeriod rule (!94) * Remove no longer needed test_revoke_smiths_them_rejoin in duniter-wot * Remove dead code (MembershipRevokedRecently) * Remove empty fn prune_revoked_memberships * Remove RevokedMembershipPrunedOn never used * Remove RevokedMembership that is always empty * Remove RevocationPeriod --- pallets/duniter-wot/src/mock.rs | 4 --- pallets/duniter-wot/src/tests.rs | 27 -------------- pallets/membership/src/lib.rs | 53 +++------------------------- pallets/membership/src/mock.rs | 2 -- pallets/membership/src/tests.rs | 9 +---- runtime/common/src/pallets_config.rs | 6 ++-- 6 files changed, 8 insertions(+), 93 deletions(-) diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index 458c40d11..56d836673 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -139,7 +139,6 @@ impl pallet_identity::Config for Test { parameter_types! { pub const MembershipPeriod: u64 = 8; pub const PendingMembershipPeriod: u64 = 3; - pub const RevocationPeriod: u64 = 4; } impl pallet_membership::Config<Instance1> for Test { @@ -153,7 +152,6 @@ impl pallet_membership::Config<Instance1> for Test { type MetaData = (); type OnEvent = DuniterWot; type PendingMembershipPeriod = PendingMembershipPeriod; - type RevocationPeriod = RevocationPeriod; } // Cert @@ -195,7 +193,6 @@ impl pallet_duniter_wot::Config<Instance2> for Test { parameter_types! { pub const SmithsMembershipPeriod: u64 = 20; pub const SmithsPendingMembershipPeriod: u64 = 3; - pub const SmithsRevocationPeriod: u64 = 4; } impl pallet_membership::Config<Instance2> for Test { @@ -209,7 +206,6 @@ impl pallet_membership::Config<Instance2> for Test { type MetaData = (); type OnEvent = SmithsSubWot; type PendingMembershipPeriod = SmithsPendingMembershipPeriod; - type RevocationPeriod = SmithsRevocationPeriod; } // SmithsCert diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs index 7e1e8e901..100d50469 100644 --- a/pallets/duniter-wot/src/tests.rs +++ b/pallets/duniter-wot/src/tests.rs @@ -143,33 +143,6 @@ fn test_smith_member_cant_revoke_its_idty() { }); } -#[test] -fn test_revoke_smiths_them_rejoin() { - new_test_ext(5, 4).execute_with(|| { - run_to_block(2); - - // Dave shoud be able to revoke his smith membership - assert_ok!(SmithsMembership::revoke_membership( - Origin::signed(4), - Some(4) - )); - - // Dave should not be able to re-request membership before the RevocationPeriod end - run_to_block(3); - assert_noop!( - SmithsMembership::request_membership(Origin::signed(4), ()), - pallet_membership::Error::<Test, crate::Instance2>::MembershipRevokedRecently - ); - - // At block #6, Dave shoud be able to request smith membership - run_to_block(6); - assert_ok!(SmithsMembership::request_membership(Origin::signed(4), ())); - - // Then, Alice should be able to send a smith cert to Dave - assert_ok!(SmithsCert::add_cert(Origin::signed(1), 1, 4)); - }); -} - #[test] fn test_create_idty_ok() { new_test_ext(5, 2).execute_with(|| { diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs index 93ff97dc6..807a4966f 100644 --- a/pallets/membership/src/lib.rs +++ b/pallets/membership/src/lib.rs @@ -81,9 +81,6 @@ pub mod pallet { #[pallet::constant] /// Maximum period (in number of blocks), where an identity can remain pending subscription. type PendingMembershipPeriod: Get<Self::BlockNumber>; - #[pallet::constant] - /// Minimum duration (in number of blocks between a revocation and a new entry request - type RevocationPeriod: Get<Self::BlockNumber>; } // GENESIS STUFFĂ‚ // @@ -134,16 +131,6 @@ pub mod pallet { pub type PendingMembershipsExpireOn<T: Config<I>, I: 'static = ()> = StorageMap<_, Twox64Concat, T::BlockNumber, Vec<T::IdtyId>, ValueQuery>; - #[pallet::storage] - #[pallet::getter(fn revoked_membership)] - pub type RevokedMembership<T: Config<I>, I: 'static = ()> = - StorageMap<_, Twox64Concat, T::IdtyId, (), OptionQuery>; - - #[pallet::storage] - #[pallet::getter(fn revoked_memberships_pruned_on)] - pub type RevokedMembershipsPrunedOn<T: Config<I>, I: 'static = ()> = - StorageMap<_, Twox64Concat, T::BlockNumber, Vec<T::IdtyId>, OptionQuery>; - // EVENTS // #[pallet::event] @@ -193,8 +180,6 @@ pub mod pallet { OriginNotAllowedToUseIdty, /// Membership request not found MembershipRequestNotFound, - /// Membership revoked recently - MembershipRevokedRecently, } // HOOKS // @@ -203,9 +188,7 @@ pub mod pallet { impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> { fn on_initialize(n: T::BlockNumber) -> Weight { if n > T::BlockNumber::zero() { - Self::expire_pending_memberships(n) - + Self::expire_memberships(n) - + Self::prune_revoked_memberships(n) + Self::expire_pending_memberships(n) + Self::expire_memberships(n) } else { 0 } @@ -307,7 +290,10 @@ pub mod pallet { let idty_id = Self::ensure_origin_and_get_idty_id(origin, maybe_idty_id)?; // Apply phase - let _ = Self::do_revoke_membership(idty_id); + if Self::remove_membership(&idty_id) { + Self::deposit_event(Event::MembershipRevoked(idty_id)); + T::OnEvent::on_event(&sp_membership::Event::MembershipRevoked(idty_id)); + } Ok(().into()) } @@ -340,9 +326,6 @@ pub mod pallet { if Membership::<T, I>::contains_key(&idty_id) { return Err(Error::<T, I>::MembershipAlreadyAcquired.into()); } - if RevokedMembership::<T, I>::contains_key(&idty_id) { - return Err(Error::<T, I>::MembershipRevokedRecently.into()); - } let block_number = frame_system::pallet::Pallet::<T>::block_number(); let expire_on = block_number + T::PendingMembershipPeriod::get(); @@ -354,21 +337,6 @@ pub mod pallet { Ok(().into()) } - pub(super) fn do_revoke_membership(idty_id: T::IdtyId) -> Weight { - if Self::remove_membership(&idty_id) { - if T::RevocationPeriod::get() > Zero::zero() { - let block_number = frame_system::pallet::Pallet::<T>::block_number(); - let pruned_on = block_number + T::RevocationPeriod::get(); - - RevokedMembership::<T, I>::insert(idty_id, ()); - RevokedMembershipsPrunedOn::<T, I>::append(pruned_on, idty_id); - } - Self::deposit_event(Event::MembershipRevoked(idty_id)); - T::OnEvent::on_event(&sp_membership::Event::MembershipRevoked(idty_id)); - } - - 0 - } fn ensure_origin_and_get_idty_id( origin: OriginFor<T>, maybe_idty_id: Option<T::IdtyId>, @@ -412,17 +380,6 @@ pub mod pallet { total_weight } - fn prune_revoked_memberships(block_number: T::BlockNumber) -> Weight { - let total_weight: Weight = 0; - - if let Some(identities_ids) = RevokedMembershipsPrunedOn::<T, I>::take(block_number) { - for idty_id in identities_ids { - RevokedMembership::<T, I>::remove(idty_id); - } - } - - total_weight - } pub(super) fn is_member_inner(idty_id: &T::IdtyId) -> bool { Membership::<T, I>::contains_key(idty_id) diff --git a/pallets/membership/src/mock.rs b/pallets/membership/src/mock.rs index e9ea07467..afbe52f97 100644 --- a/pallets/membership/src/mock.rs +++ b/pallets/membership/src/mock.rs @@ -80,7 +80,6 @@ impl system::Config for Test { parameter_types! { pub const MembershipPeriod: BlockNumber = 5; pub const PendingMembershipPeriod: BlockNumber = 3; - pub const RevocationPeriod: BlockNumber = 4; } impl pallet_membership::Config for Test { @@ -94,7 +93,6 @@ impl pallet_membership::Config for Test { type MetaData = (); type OnEvent = (); type PendingMembershipPeriod = PendingMembershipPeriod; - type RevocationPeriod = RevocationPeriod; } // Build genesis storage according to the mock runtime. diff --git a/pallets/membership/src/tests.rs b/pallets/membership/src/tests.rs index 67fa62fd0..723605be8 100644 --- a/pallets/membership/src/tests.rs +++ b/pallets/membership/src/tests.rs @@ -113,14 +113,7 @@ fn test_membership_revocation() { RuntimeEvent::DefaultMembership(Event::MembershipRevoked(0)) ); - // Membership 0 can't request membership before the end of RevokePeriod (1 + 4 = 5) - run_to_block(2); - assert_eq!( - DefaultMembership::request_membership(Origin::signed(0), ()), - Err(Error::<Test, _>::MembershipRevokedRecently.into()) - ); - - // Membership 0 can request membership after the end of RevokePeriod (1 + 4 = 5) + // Membership 0 can re-request membership run_to_block(5); assert_ok!(DefaultMembership::request_membership(Origin::signed(0), ()),); assert_eq!( diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index 7bf469f49..5950b43f9 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -444,7 +444,7 @@ macro_rules! pallets_config { } impl pallet_membership::Config<frame_support::instances::Instance1> for Runtime { - type IsIdtyAllowedToClaimMembership = Wot; + type IsIdtyAllowedToClaimMembership = Wot; type IsIdtyAllowedToRenewMembership = Wot; type IsIdtyAllowedToRequestMembership = Wot; type Event = Event; @@ -454,7 +454,6 @@ macro_rules! pallets_config { type MetaData = (); type OnEvent = OnMembershipEventHandler<Wot, Runtime>; type PendingMembershipPeriod = PendingMembershipPeriod; - type RevocationPeriod = frame_support::traits::ConstU32<0>; } impl pallet_certification::Config<Instance1> for Runtime { @@ -481,7 +480,7 @@ macro_rules! pallets_config { } impl pallet_membership::Config<Instance2> for Runtime { - type IsIdtyAllowedToClaimMembership = SmithsSubWot; + type IsIdtyAllowedToClaimMembership = SmithsSubWot; type IsIdtyAllowedToRenewMembership = SmithsSubWot; type IsIdtyAllowedToRequestMembership = SmithsSubWot; type Event = Event; @@ -491,7 +490,6 @@ macro_rules! pallets_config { type MetaData = SmithsMembershipMetaData<opaque::SessionKeysWrapper>; type OnEvent = OnSmithMembershipEventHandler<SmithsSubWot, Runtime>; type PendingMembershipPeriod = SmithPendingMembershipPeriod; - type RevocationPeriod = frame_support::traits::ConstU32<0>; } impl pallet_certification::Config<Instance2> for Runtime { -- GitLab