From ed46bb933e32d32d8207a9ef6000d243e04ad6e4 Mon Sep 17 00:00:00 2001
From: bgallois <benjamin@gallois.cc>
Date: Wed, 6 Mar 2024 18:29:26 +0100
Subject: [PATCH] add handlers documentation

---
 primitives/membership/src/traits.rs |  6 ++++++
 runtime/common/src/handlers.rs      | 30 ++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/primitives/membership/src/traits.rs b/primitives/membership/src/traits.rs
index f67655299..2e5a30930 100644
--- a/primitives/membership/src/traits.rs
+++ b/primitives/membership/src/traits.rs
@@ -16,8 +16,11 @@
 
 use frame_support::pallet_prelude::*;
 
+/// A trait defining operations for checking if membership-related operations are allowed.
 pub trait CheckMembershipOpAllowed<IdtyId> {
+    /// Checks if adding a membership is allowed.
     fn check_add_membership(idty_id: IdtyId) -> Result<(), DispatchError>;
+    /// Checks if renewing a membership is allowed.
     fn check_renew_membership(idty_id: IdtyId) -> Result<(), DispatchError>;
 }
 
@@ -31,11 +34,14 @@ impl<IdtyId> CheckMembershipOpAllowed<IdtyId> for () {
     }
 }
 
+/// A trait defining behavior for handling new memberships and membership renewals.
 pub trait OnNewMembership<IdtyId> {
+    /// Called when a new membership is created.
     fn on_created(idty_index: &IdtyId);
     fn on_renewed(idty_index: &IdtyId);
 }
 
+/// Called when an existing membership is renewed.
 pub trait OnRemoveMembership<IdtyId> {
     fn on_removed(idty_index: &IdtyId) -> Weight;
 }
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index 2c71468c2..54eec5cb4 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -20,7 +20,8 @@ use frame_support::pallet_prelude::Weight;
 use frame_support::traits::UnfilteredDispatchable;
 use pallet_smith_members::SmithRemovalReason;
 
-// new session handler
+/// OnNewSession handler for the runtime calling all the implementation
+/// of OnNewSession
 pub struct OnNewSessionHandler<Runtime>(core::marker::PhantomData<Runtime>);
 impl<Runtime> pallet_authority_members::traits::OnNewSession for OnNewSessionHandler<Runtime>
 where
@@ -32,13 +33,12 @@ where
     }
 }
 
-/// New membership event runtime handler
+/// Runtime handler for OnNewMembership, calling all implementations of
+/// OnNewMembership and implementing logic at the runtime level.
 pub struct OnNewMembershipHandler<Runtime>(core::marker::PhantomData<Runtime>);
 impl<
         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<Runtime>
@@ -46,7 +46,8 @@ impl<
     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
+
+        // 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 {
@@ -60,12 +61,14 @@ impl<
     fn on_renewed(_idty_index: &IdtyIndex) {}
 }
 
-/// Remove membership event runtime handler
+/// Runtime handler for OnRemoveMembership, calling all implementations of
+/// OnRemoveMembership and implementing logic at the runtime level.
+/// As the weight accounting is not trivial in this handler, the weight is
+/// done at the handler level.
 pub struct OnRemoveMembershipHandler<Runtime>(core::marker::PhantomData<Runtime>);
 impl<
         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,
@@ -74,11 +77,12 @@ impl<
     fn on_removed(idty_index: &IdtyIndex) -> Weight {
         // 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);
         };
 
-        // when membership is removed, call on_removed_member handler which auto claims UD
+        // 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() {
@@ -89,12 +93,15 @@ impl<
                 );
             }
         }
-        weight += pallet_smith_members::Pallet::<Runtime>::on_removed_wot_member(*idty_index);
-        weight
+
+        // When membership is removed, also remove from smith member.
+        weight.saturating_add(
+            pallet_smith_members::Pallet::<Runtime>::on_removed_wot_member(*idty_index),
+        )
     }
 }
 
-// spend treasury handler
+/// Runtime handler for TreasurySpendFunds.
 pub struct TreasurySpendFunds<Runtime>(core::marker::PhantomData<Runtime>);
 impl<Runtime> pallet_treasury::SpendFunds<Runtime> for TreasurySpendFunds<Runtime>
 where
@@ -110,6 +117,7 @@ where
     }
 }
 
+/// Runtime handler for OnSmithDelete.
 pub struct OnSmithDeletedHandler<Runtime>(core::marker::PhantomData<Runtime>);
 impl<Runtime> pallet_smith_members::traits::OnSmithDelete<Runtime::MemberId>
     for OnSmithDeletedHandler<Runtime>
-- 
GitLab