From dcb34e4c1923465a72ecf82fba4c7f959b533b00 Mon Sep 17 00:00:00 2001
From: bgallois <benjamin@gallois.cc>
Date: Wed, 6 Mar 2024 15:39:14 +0100
Subject: [PATCH] simplify Membership handlers

---
 Cargo.lock                           |  1 +
 pallets/duniter-wot/src/lib.rs       |  8 +++---
 pallets/duniter-wot/src/mock.rs      |  3 ++-
 pallets/identity/src/lib.rs          |  3 +--
 pallets/membership/src/mock.rs       |  3 ++-
 primitives/membership/src/lib.rs     | 13 +++-------
 primitives/membership/src/traits.rs  |  8 ++----
 runtime/common/Cargo.toml            |  4 +++
 runtime/common/src/handlers.rs       | 37 +++++++++++-----------------
 runtime/common/src/pallets_config.rs |  4 +--
 10 files changed, 36 insertions(+), 48 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 0da887cda..59fb60db4 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1472,6 +1472,7 @@ dependencies = [
  "pallet-collective",
  "pallet-distance",
  "pallet-duniter-account",
+ "pallet-duniter-wot",
  "pallet-grandpa",
  "pallet-identity",
  "pallet-im-online",
diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index 6f828b72a..b84102845 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -200,15 +200,13 @@ impl<T: Config> sp_membership::traits::OnNewMembership<IdtyIndex> for Pallet<T>
 where
     T: pallet_membership::Config,
 {
-    fn on_created(idty_index: &IdtyIndex) -> Weight {
+    fn on_created(idty_index: &IdtyIndex) {
         // when main membership is acquired, tell identity
         // (only used on first membership acquiry)
-        pallet_identity::Pallet::<T>::membership_added(*idty_index)
+        pallet_identity::Pallet::<T>::membership_added(*idty_index);
     }
 
-    fn on_renewed(_idty_index: &IdtyIndex) -> Weight {
-        Weight::zero()
-    }
+    fn on_renewed(_idty_index: &IdtyIndex) {}
 }
 
 // implement membership event handler
diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs
index c7be1ef55..29254167f 100644
--- a/pallets/duniter-wot/src/mock.rs
+++ b/pallets/duniter-wot/src/mock.rs
@@ -147,7 +147,8 @@ impl pallet_membership::Config for Test {
     type IdtyIdOf = IdentityIndexOf<Self>;
     type MembershipPeriod = MembershipPeriod;
     type MembershipRenewalPeriod = MembershipRenewalPeriod;
-    type OnEvent = DuniterWot;
+    type OnNewMembership = DuniterWot;
+    type OnRemoveMembership = DuniterWot;
     type RuntimeEvent = RuntimeEvent;
     type WeightInfo = ();
 }
diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs
index aa6a6c5a8..6e5c07c58 100644
--- a/pallets/identity/src/lib.rs
+++ b/pallets/identity/src/lib.rs
@@ -638,7 +638,7 @@ pub mod pallet {
         // when an identity becomes member, update its status
         // unschedule identity action, this falls back to membership scheduling
         // no identity schedule while membership is active
-        pub fn membership_added(idty_index: T::IdtyIndex) -> Weight {
+        pub fn membership_added(idty_index: T::IdtyIndex) {
             if let Some(mut idty_value) = Identities::<T>::get(idty_index) {
                 Self::unschedule_identity_change(idty_index, idty_value.next_scheduled);
                 idty_value.next_scheduled = BlockNumberFor::<T>::zero();
@@ -649,7 +649,6 @@ pub mod pallet {
                 idty_value.status = IdtyStatus::Member;
                 <Identities<T>>::insert(idty_index, idty_value);
             }
-            T::WeightInfo::membership_added()
             // else should not happen
         }
 
diff --git a/pallets/membership/src/mock.rs b/pallets/membership/src/mock.rs
index 27070cd5f..5fb5d29ec 100644
--- a/pallets/membership/src/mock.rs
+++ b/pallets/membership/src/mock.rs
@@ -85,7 +85,8 @@ impl pallet_membership::Config for Test {
     type IdtyIdOf = ConvertInto;
     type MembershipPeriod = MembershipPeriod;
     type MembershipRenewalPeriod = MembershipRenewalPeriod;
-    type OnEvent = ();
+    type OnNewMembership = ();
+    type OnRemoveMembership = ();
     type RuntimeEvent = RuntimeEvent;
     type WeightInfo = ();
 }
diff --git a/primitives/membership/src/lib.rs b/primitives/membership/src/lib.rs
index e14fe610d..e74247356 100644
--- a/primitives/membership/src/lib.rs
+++ b/primitives/membership/src/lib.rs
@@ -61,18 +61,13 @@ use impl_trait_for_tuples::impl_for_tuples;
 // use frame_system::pallet_prelude::*;
 
 #[impl_for_tuples(5)]
-#[allow(clippy::let_and_return)]
 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_created(idty_index)); )* );
-        weight
+    fn on_created(idty_index: &IdtyId) {
+        for_tuples!( #(Tuple::on_created(idty_index); )* );
     }
 
-    fn on_renewed(idty_index: &IdtyId) -> Weight {
-        let mut weight = Weight::zero();
-        for_tuples!( #( weight = weight.saturating_add(Tuple::on_renewed(idty_index)); )* );
-        weight
+    fn on_renewed(idty_index: &IdtyId) {
+        for_tuples!( #( Tuple::on_renewed(idty_index); )* );
     }
 }
 
diff --git a/primitives/membership/src/traits.rs b/primitives/membership/src/traits.rs
index cd1e5f853..f67655299 100644
--- a/primitives/membership/src/traits.rs
+++ b/primitives/membership/src/traits.rs
@@ -32,18 +32,14 @@ impl<IdtyId> CheckMembershipOpAllowed<IdtyId> for () {
 }
 
 pub trait OnNewMembership<IdtyId> {
-    fn on_created(idty_index: &IdtyId) -> Weight;
-    fn on_renewed(idty_index: &IdtyId) -> Weight;
+    fn on_created(idty_index: &IdtyId);
+    fn on_renewed(idty_index: &IdtyId);
 }
 
 pub trait OnRemoveMembership<IdtyId> {
     fn on_removed(idty_index: &IdtyId) -> Weight;
 }
 
-// impl<IdtyId> OnEvent<IdtyId> for () {
-//     fn on_event(_: &crate::Event<IdtyId>) {}
-// }
-
 pub trait MembersCount {
     fn members_count() -> u32;
 }
diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml
index 4dae70831..aafe5a6b1 100644
--- a/runtime/common/Cargo.toml
+++ b/runtime/common/Cargo.toml
@@ -25,6 +25,7 @@ runtime-benchmarks = [
 	"pallet-collective/runtime-benchmarks",
 	"pallet-distance/runtime-benchmarks",
 	"pallet-duniter-account/runtime-benchmarks",
+	"pallet-duniter-wot/runtime-benchmarks",
 	"pallet-grandpa/runtime-benchmarks",
 	"pallet-identity/runtime-benchmarks",
 	"pallet-im-online/runtime-benchmarks",
@@ -61,6 +62,7 @@ std = [
 	"pallet-collective/std",
 	"pallet-distance/std",
 	"pallet-duniter-account/std",
+	"pallet-duniter-wot/std",
 	"pallet-grandpa/std",
 	"pallet-identity/std",
 	"pallet-im-online/std",
@@ -103,6 +105,7 @@ try-runtime = [
 	"pallet-collective/try-runtime",
 	"pallet-distance/try-runtime",
 	"pallet-duniter-account/try-runtime",
+	"pallet-duniter-wot/try-runtime",
 	"pallet-grandpa/try-runtime",
 	"pallet-identity/try-runtime",
 	"pallet-im-online/try-runtime",
@@ -141,6 +144,7 @@ pallet-certification = { workspace = true }
 pallet-collective = { workspace = true }
 pallet-distance = { workspace = true }
 pallet-duniter-account = { workspace = true }
+pallet-duniter-wot = { workspace = true }
 pallet-grandpa = { workspace = true }
 pallet-identity = { workspace = true }
 pallet-im-online = { workspace = true }
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index 3f88fa64a..2c71468c2 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -32,23 +32,20 @@ where
     }
 }
 
-// membership event runtime handler
-pub struct OnNewMembershipHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>);
+/// New membership event runtime handler
+pub struct OnNewMembershipHandler<Runtime>(core::marker::PhantomData<Runtime>);
 impl<
-        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_duniter_wot::Config
             + pallet_universal_dividend::Config,
-    > sp_membership::traits::OnNewMembership<IdtyIndex> for OnNewMembershipHandler<Inner, Runtime>
+    > sp_membership::traits::OnNewMembership<IdtyIndex> for OnNewMembershipHandler<Runtime>
 {
-    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);
-        };
-
+    fn on_created(idty_index: &IdtyIndex) {
+        // duniter-wot related actions
+        pallet_duniter_wot::Pallet::<Runtime>::on_created(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 {
@@ -58,29 +55,25 @@ impl<
                 };
             }
         });
-        add_db_reads_writes(1, 1);
-        weight.saturating_add(Inner::on_created(idty_index))
     }
 
-    fn on_renewed(_idty_index: &IdtyIndex) -> Weight {
-        Weight::zero()
-    }
+    fn on_renewed(_idty_index: &IdtyIndex) {}
 }
 
-// membership event runtime handler
-pub struct OnRemoveMembershipHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>);
+/// Remove membership event runtime handler
+pub struct OnRemoveMembershipHandler<Runtime>(core::marker::PhantomData<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_duniter_wot::Config
             + pallet_universal_dividend::Config,
-    > sp_membership::traits::OnRemoveMembership<IdtyIndex>
-    for OnRemoveMembershipHandler<Inner, Runtime>
+    > sp_membership::traits::OnRemoveMembership<IdtyIndex> for OnRemoveMembershipHandler<Runtime>
 {
     fn on_removed(idty_index: &IdtyIndex) -> Weight {
-        let mut weight = Weight::zero();
+        // duniter-wot related actions
+        let mut weight = pallet_duniter_wot::Pallet::<Runtime>::on_removed(idty_index);
         let mut add_db_reads_writes = |reads, writes| {
             weight += crate::constants::DbWeight::get().reads_writes(reads, writes);
         };
@@ -97,7 +90,7 @@ impl<
             }
         }
         weight += pallet_smith_members::Pallet::<Runtime>::on_removed_wot_member(*idty_index);
-        weight.saturating_add(Inner::on_removed(idty_index))
+        weight
     }
 }
 
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index 45d73571a..c713b5bfb 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/runtime/common/src/pallets_config.rs
@@ -499,8 +499,8 @@ type RuntimeFreezeReason = ();
             type AccountIdOf = common_runtime::providers::IdentityAccountIdProvider<Self>;
             type MembershipPeriod = MembershipPeriod;
             type MembershipRenewalPeriod = MembershipRenewalPeriod;
-            type OnNewMembership = (OnNewMembershipHandler<Wot, Runtime>, Wot);
-            type OnRemoveMembership = (OnRemoveMembershipHandler<Wot, Runtime>);
+            type OnNewMembership = OnNewMembershipHandler<Runtime>;
+            type OnRemoveMembership = OnRemoveMembershipHandler<Runtime>;
             type RuntimeEvent = RuntimeEvent;
             type WeightInfo = common_runtime::weights::pallet_membership::WeightInfo<Runtime>;
             #[cfg(feature = "runtime-benchmarks")]
-- 
GitLab