From 22ea5fa0d4da62d3d298da65fc54f36afaad6887 Mon Sep 17 00:00:00 2001
From: bgallois <benjamin@gallois.cc>
Date: Wed, 6 Mar 2024 13:41:09 +0100
Subject: [PATCH] split OnEvent membership handler

---
 pallets/duniter-wot/src/lib.rs       | 34 ++++++-----
 pallets/membership/src/lib.rs        | 14 ++---
 primitives/membership/src/lib.rs     | 22 ++++++-
 primitives/membership/src/traits.rs  |  9 ++-
 runtime/common/src/handlers.rs       | 86 +++++++++++++++++-----------
 runtime/common/src/pallets_config.rs |  3 +-
 6 files changed, 106 insertions(+), 62 deletions(-)

diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index 6d0f288e5..6f828b72a 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -196,23 +196,29 @@ impl<T: Config> sp_membership::traits::CheckMembershipOpAllowed<IdtyIndex> for P
 }
 
 // implement membership event handler
-impl<T: Config> sp_membership::traits::OnEvent<IdtyIndex> for Pallet<T>
+impl<T: Config> sp_membership::traits::OnNewMembership<IdtyIndex> for Pallet<T>
 where
     T: pallet_membership::Config,
 {
-    fn on_event(membership_event: &sp_membership::Event<IdtyIndex>) -> Weight {
-        match membership_event {
-            sp_membership::Event::<IdtyIndex>::MembershipAdded(idty_index) => {
-                // when main membership is acquired, tell identity
-                // (only used on first membership acquiry)
-                pallet_identity::Pallet::<T>::membership_added(*idty_index)
-            }
-            sp_membership::Event::<IdtyIndex>::MembershipRemoved(idty_index) => {
-                // when main membership is lost, tell identity
-                pallet_identity::Pallet::<T>::membership_removed(*idty_index)
-            }
-            sp_membership::Event::<IdtyIndex>::MembershipRenewed(_) => Weight::zero(),
-        }
+    fn on_created(idty_index: &IdtyIndex) -> Weight {
+        // when main membership is acquired, tell identity
+        // (only used on first membership acquiry)
+        pallet_identity::Pallet::<T>::membership_added(*idty_index)
+    }
+
+    fn on_renewed(_idty_index: &IdtyIndex) -> Weight {
+        Weight::zero()
+    }
+}
+
+// implement membership event handler
+impl<T: Config> sp_membership::traits::OnRemoveMembership<IdtyIndex> for Pallet<T>
+where
+    T: pallet_membership::Config,
+{
+    fn on_removed(idty_index: &IdtyIndex) -> Weight {
+        // when main membership is lost, tell identity
+        pallet_identity::Pallet::<T>::membership_removed(*idty_index)
     }
 }
 
diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs
index 4217893cc..06b127ed0 100644
--- a/pallets/membership/src/lib.rs
+++ b/pallets/membership/src/lib.rs
@@ -99,8 +99,10 @@ pub mod pallet {
         // i.e. asking for distance evaluation
         #[pallet::constant]
         type MembershipRenewalPeriod: Get<BlockNumberFor<Self>>;
-        /// On event handler
-        type OnEvent: OnEvent<Self::IdtyId>;
+        /// On new and renew membership handler
+        type OnNewMembership: OnNewMembership<Self::IdtyId>;
+        /// On revoked and removed membership handler
+        type OnRemoveMembership: OnRemoveMembership<Self::IdtyId>;
         /// 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 WeightInfo: WeightInfo;
@@ -272,7 +274,7 @@ pub mod pallet {
                 member: idty_id,
                 expire_on,
             });
-            T::OnEvent::on_event(&sp_membership::Event::MembershipAdded(idty_id));
+            T::OnNewMembership::on_created(&idty_id);
         }
 
         /// perform membership renewal
@@ -286,7 +288,7 @@ pub mod pallet {
                 member: idty_id,
                 expire_on,
             });
-            T::OnEvent::on_event(&sp_membership::Event::MembershipRenewed(idty_id));
+            T::OnNewMembership::on_renewed(&idty_id);
         }
 
         /// perform membership removal
@@ -304,9 +306,7 @@ pub mod pallet {
                     member: idty_id,
                     reason,
                 });
-                weight = weight.saturating_add(T::OnEvent::on_event(
-                    &sp_membership::Event::MembershipRemoved(idty_id),
-                ));
+                weight = weight.saturating_add(T::OnRemoveMembership::on_removed(&idty_id));
             }
             weight
         }
diff --git a/primitives/membership/src/lib.rs b/primitives/membership/src/lib.rs
index d2ef9952a..e14fe610d 100644
--- a/primitives/membership/src/lib.rs
+++ b/primitives/membership/src/lib.rs
@@ -62,10 +62,26 @@ use impl_trait_for_tuples::impl_for_tuples;
 
 #[impl_for_tuples(5)]
 #[allow(clippy::let_and_return)]
-impl<IdtyId> traits::OnEvent<IdtyId> for Tuple {
-    fn on_event(event: &Event<IdtyId>) -> Weight {
+impl<IdtyId> traits::OnNewMembership<IdtyId> for Tuple {
+    fn on_created(idty_index: &IdtyId) -> Weight {
         let mut weight = Weight::zero();
-        for_tuples!( #( weight = weight.saturating_add(Tuple::on_event(event)); )* );
+        for_tuples!( #( weight = weight.saturating_add(Tuple::on_created(idty_index)); )* );
+        weight
+    }
+
+    fn on_renewed(idty_index: &IdtyId) -> Weight {
+        let mut weight = Weight::zero();
+        for_tuples!( #( weight = weight.saturating_add(Tuple::on_renewed(idty_index)); )* );
+        weight
+    }
+}
+
+#[impl_for_tuples(5)]
+#[allow(clippy::let_and_return)]
+impl<IdtyId> traits::OnRemoveMembership<IdtyId> for Tuple {
+    fn on_removed(idty_index: &IdtyId) -> Weight {
+        let mut weight = Weight::zero();
+        for_tuples!( #( weight = weight.saturating_add(Tuple::on_removed(idty_index)); )* );
         weight
     }
 }
diff --git a/primitives/membership/src/traits.rs b/primitives/membership/src/traits.rs
index 633acfdad..cd1e5f853 100644
--- a/primitives/membership/src/traits.rs
+++ b/primitives/membership/src/traits.rs
@@ -31,8 +31,13 @@ impl<IdtyId> CheckMembershipOpAllowed<IdtyId> for () {
     }
 }
 
-pub trait OnEvent<IdtyId> {
-    fn on_event(event: &crate::Event<IdtyId>) -> Weight;
+pub trait OnNewMembership<IdtyId> {
+    fn on_created(idty_index: &IdtyId) -> Weight;
+    fn on_renewed(idty_index: &IdtyId) -> Weight;
+}
+
+pub trait OnRemoveMembership<IdtyId> {
+    fn on_removed(idty_index: &IdtyId) -> Weight;
 }
 
 // impl<IdtyId> OnEvent<IdtyId> for () {
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index e02d2870a..3f88fa64a 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -33,55 +33,71 @@ where
 }
 
 // membership event runtime handler
-pub struct OnMembershipEventHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>);
+pub struct OnNewMembershipHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>);
 impl<
-        Inner: sp_membership::traits::OnEvent<IdtyIndex>,
+        Inner: sp_membership::traits::OnNewMembership<IdtyIndex>,
         Runtime: frame_system::Config<AccountId = AccountId>
             + pallet_identity::Config<IdtyData = IdtyData, IdtyIndex = IdtyIndex>
             + pallet_membership::Config
             + pallet_smith_members::Config<IdtyIndex = IdtyIndex>
             + pallet_universal_dividend::Config,
-    > sp_membership::traits::OnEvent<IdtyIndex> for OnMembershipEventHandler<Inner, Runtime>
+    > sp_membership::traits::OnNewMembership<IdtyIndex> for OnNewMembershipHandler<Inner, Runtime>
 {
-    fn on_event(membership_event: &sp_membership::Event<IdtyIndex>) -> Weight {
+    fn on_created(idty_index: &IdtyIndex) -> Weight {
         let mut weight = Weight::zero();
         let mut add_db_reads_writes = |reads, writes| {
             weight += crate::constants::DbWeight::get().reads_writes(reads, writes);
         };
 
-        (match membership_event {
-            // when membership is removed, call on_removed_member handler which auto claims UD
-            sp_membership::Event::MembershipRemoved(idty_index) => {
-                if let Some(idty_value) = pallet_identity::Identities::<Runtime>::get(idty_index) {
-                    add_db_reads_writes(1, 0);
-                    if let Some(first_ud_index) = idty_value.data.first_eligible_ud.into() {
-                        add_db_reads_writes(1, 0);
-                        weight += pallet_universal_dividend::Pallet::<Runtime>::on_removed_member(
-                            first_ud_index,
-                            &idty_value.owner_key,
-                        );
-                    }
-                }
-                weight +=
-                    pallet_smith_members::Pallet::<Runtime>::on_removed_wot_member(*idty_index);
+        // when main membership is acquired, it starts getting right to UD
+        pallet_identity::Identities::<Runtime>::mutate_exists(idty_index, |idty_val_opt| {
+            if let Some(ref mut idty_val) = idty_val_opt {
+                idty_val.data = IdtyData {
+                    first_eligible_ud:
+                        pallet_universal_dividend::Pallet::<Runtime>::init_first_eligible_ud(),
+                };
             }
-            // when main membership is acquired, it starts getting right to UD
-            sp_membership::Event::MembershipAdded(idty_index) => {
-                pallet_identity::Identities::<Runtime>::mutate_exists(idty_index, |idty_val_opt| {
-                    if let Some(ref mut idty_val) = idty_val_opt {
-                        idty_val.data = IdtyData {
-                            first_eligible_ud:
-                                pallet_universal_dividend::Pallet::<Runtime>::init_first_eligible_ud(
-                                ),
-                        };
-                    }
-                });
-                add_db_reads_writes(1, 1);
-            }
-            // in other case, ther is nothing to do
-            sp_membership::Event::MembershipRenewed(_) => (),
         });
-        weight.saturating_add(Inner::on_event(membership_event))
+        add_db_reads_writes(1, 1);
+        weight.saturating_add(Inner::on_created(idty_index))
+    }
+
+    fn on_renewed(_idty_index: &IdtyIndex) -> Weight {
+        Weight::zero()
+    }
+}
+
+// membership event runtime handler
+pub struct OnRemoveMembershipHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>);
+impl<
+        Inner: sp_membership::traits::OnRemoveMembership<IdtyIndex>,
+        Runtime: frame_system::Config<AccountId = AccountId>
+            + pallet_identity::Config<IdtyData = IdtyData, IdtyIndex = IdtyIndex>
+            + pallet_membership::Config
+            + pallet_smith_members::Config<IdtyIndex = IdtyIndex>
+            + pallet_universal_dividend::Config,
+    > sp_membership::traits::OnRemoveMembership<IdtyIndex>
+    for OnRemoveMembershipHandler<Inner, Runtime>
+{
+    fn on_removed(idty_index: &IdtyIndex) -> Weight {
+        let mut weight = Weight::zero();
+        let mut add_db_reads_writes = |reads, writes| {
+            weight += crate::constants::DbWeight::get().reads_writes(reads, writes);
+        };
+
+        // when membership is removed, call on_removed_member handler which auto claims UD
+        if let Some(idty_value) = pallet_identity::Identities::<Runtime>::get(idty_index) {
+            add_db_reads_writes(1, 0);
+            if let Some(first_ud_index) = idty_value.data.first_eligible_ud.into() {
+                add_db_reads_writes(1, 0);
+                weight += pallet_universal_dividend::Pallet::<Runtime>::on_removed_member(
+                    first_ud_index,
+                    &idty_value.owner_key,
+                );
+            }
+        }
+        weight += pallet_smith_members::Pallet::<Runtime>::on_removed_wot_member(*idty_index);
+        weight.saturating_add(Inner::on_removed(idty_index))
     }
 }
 
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index f2a89104c..45d73571a 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/runtime/common/src/pallets_config.rs
@@ -499,7 +499,8 @@ type RuntimeFreezeReason = ();
             type AccountIdOf = common_runtime::providers::IdentityAccountIdProvider<Self>;
             type MembershipPeriod = MembershipPeriod;
             type MembershipRenewalPeriod = MembershipRenewalPeriod;
-            type OnEvent = (OnMembershipEventHandler<Wot, Runtime>, Wot);
+            type OnNewMembership = (OnNewMembershipHandler<Wot, Runtime>, Wot);
+            type OnRemoveMembership = (OnRemoveMembershipHandler<Wot, Runtime>);
             type RuntimeEvent = RuntimeEvent;
             type WeightInfo = common_runtime::weights::pallet_membership::WeightInfo<Runtime>;
             #[cfg(feature = "runtime-benchmarks")]
-- 
GitLab