From 8c0a202b72a87713a80f949b00f5065f8aea4c2a Mon Sep 17 00:00:00 2001
From: bgallois <benjamin@gallois.cc>
Date: Thu, 7 Mar 2024 14:46:34 +0100
Subject: [PATCH] add docs for handlers

---
 pallets/duniter-wot/src/lib.rs      | 115 +++++++++++++++++-----------
 pallets/identity/src/traits.rs      |  14 +++-
 pallets/quota/src/lib.rs            |  18 +++--
 primitives/membership/src/traits.rs |   6 +-
 4 files changed, 102 insertions(+), 51 deletions(-)

diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index 4ff1bfce9..bf79348ae 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -104,28 +104,34 @@ pub mod pallet {
     }
 }
 
-// implement identity call checks
+/// Implementing identity call allowance check for the pallet.
 impl<AccountId, T: Config> pallet_identity::traits::CheckIdtyCallAllowed<T> for Pallet<T>
 where
     T: frame_system::Config<AccountId = AccountId> + pallet_membership::Config,
 {
-    // identity creation checks
+    /// Checks if identity creation is allowed.
+    /// This implementation checks the following:
+    ///
+    /// - Whether the identity has the right to create an identity.
+    /// - Whether the issuer can emit a certification.
+    /// - Whether the issuer respect creation period.
     fn check_create_identity(creator: IdtyIndex) -> Result<(), DispatchError> {
         let cert_meta = pallet_certification::Pallet::<T>::idty_cert_meta(creator);
-        // perform all checks
-        // 1. check that identity has the right to create an identity
-        // identity can be member with 5 certifications and still not reach identity creation threshold which could be higher (6, 7...)
+
+        // 1. Check that the identity has the right to create an identity
+        // Identity can be a member with 5 certifications and still not reach the identity creation threshold, which could be higher (6, 7...)
         ensure!(
             cert_meta.received_count >= T::MinCertForCreateIdtyRight::get(),
             Error::<T>::NotEnoughReceivedCertsToCreateIdty
         );
-        // 2. check that issuer can emit one more certification
-        // (this is only a partial check)
+
+        // 2. Check that the issuer can emit one more certification (partial check)
         ensure!(
             cert_meta.issued_count < T::MaxByIssuer::get(),
             Error::<T>::MaxEmittedCertsReached
         );
-        // 3. check that issuer respects certification creation period
+
+        // 3. Check that the issuer respects certification creation period
         ensure!(
             cert_meta.next_issuable_on <= frame_system::pallet::Pallet::<T>::block_number(),
             Error::<T>::IdtyCreationPeriodNotRespected
@@ -134,16 +140,18 @@ where
     }
 }
 
-// implement cert call checks
+/// Implementing certification allowance check for the pallet.
 impl<T: Config> pallet_certification::traits::CheckCertAllowed<IdtyIndex> for Pallet<T> {
-    // check the following:
-    // - issuer has identity
-    // - issuer identity is member
-    // - receiver has identity
-    // - receiver identity is confirmed and not revoked
+    /// Checks if certification is allowed.
+    /// This implementation checks the following:
+    ///
+    /// - Whether the issuer has an identity.
+    /// - Whether the issuer's identity is a member.
+    /// - Whether the receiver has an identity.
+    /// - Whether the receiver's identity is confirmed and not revoked.
     fn check_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> Result<(), DispatchError> {
-        // issuer checks
-        // ensure issuer is member
+        // Issuer checks
+        // Ensure issuer is a member
         let issuer_data =
             pallet_identity::Pallet::<T>::identity(issuer).ok_or(Error::<T>::IdtyNotFound)?;
         ensure!(
@@ -151,8 +159,8 @@ impl<T: Config> pallet_certification::traits::CheckCertAllowed<IdtyIndex> for Pa
             Error::<T>::IssuerNotMember
         );
 
-        // receiver checks
-        // ensure receiver identity is confirmed and not revoked
+        // Receiver checks
+        // Ensure receiver identity is confirmed and not revoked
         let receiver_data =
             pallet_identity::Pallet::<T>::identity(receiver).ok_or(Error::<T>::IdtyNotFound)?;
         ensure!(
@@ -165,10 +173,14 @@ impl<T: Config> pallet_certification::traits::CheckCertAllowed<IdtyIndex> for Pa
     }
 }
 
-// implement membership call checks
+/// Implementing membership operation checks for the pallet.
 impl<T: Config> sp_membership::traits::CheckMembershipOpAllowed<IdtyIndex> for Pallet<T> {
+    /// This implementation checks the following:
+    ///
+    /// - Whether the identity's status is unvalidated or not a member.
+    /// - The count of certifications associated with the identity.
     fn check_add_membership(idty_index: IdtyIndex) -> Result<(), DispatchError> {
-        // check identity status
+        // Check identity status
         let idty_value =
             pallet_identity::Pallet::<T>::identity(idty_index).ok_or(Error::<T>::IdtyNotFound)?;
         ensure!(
@@ -176,60 +188,65 @@ impl<T: Config> sp_membership::traits::CheckMembershipOpAllowed<IdtyIndex> for P
                 || idty_value.status == IdtyStatus::NotMember,
             Error::<T>::TargetStatusInvalid
         );
-        // check cert count
+
+        // Check certificate count
         check_cert_count::<T>(idty_index)?;
         Ok(())
     }
 
-    // membership renewal is only possible when identity is member (otherwise it should claim again)
+    /// This implementation checks the following:
+    ///
+    /// - Whether the identity's status is member.
+    ///
+    /// Note: There is no need to check certification count since losing certifications makes membership expire.
+    /// Membership renewal is only possible when identity is member.
     fn check_renew_membership(idty_index: IdtyIndex) -> Result<(), DispatchError> {
-        // check identity status
         let idty_value =
             pallet_identity::Pallet::<T>::identity(idty_index).ok_or(Error::<T>::IdtyNotFound)?;
         ensure!(
             idty_value.status == IdtyStatus::Member,
             Error::<T>::TargetStatusInvalid
         );
-        // no need to check certification count since loosing certifications make membership expire
         Ok(())
     }
 }
 
-// implement membership event handler
+/// Implementing membership event handling for the pallet.
 impl<T: Config> sp_membership::traits::OnNewMembership<IdtyIndex> for Pallet<T>
 where
     T: pallet_membership::Config,
 {
+    /// This implementation notifies the identity pallet when a main membership is acquired.
+    /// It is only used on the first membership acquisition.
     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);
     }
 
     fn on_renewed(_idty_index: &IdtyIndex) {}
 }
 
-// implement membership event handler
+/// Implementing membership removal event handling for the pallet.
 impl<T: Config> sp_membership::traits::OnRemoveMembership<IdtyIndex> for Pallet<T>
 where
     T: pallet_membership::Config,
 {
+    /// This implementation notifies the identity pallet when a main membership is lost.
     fn on_removed(idty_index: &IdtyIndex) -> Weight {
-        // when main membership is lost, tell identity
         pallet_identity::Pallet::<T>::membership_removed(*idty_index)
     }
 }
 
-// implement identity event handler
+/// Implementing the identity event handler for the pallet.
 impl<T: Config> pallet_identity::traits::OnNewIdty<T> for Pallet<T> {
+    /// This implementation adds a certificate when a new identity is created.
     fn on_created(idty_index: &IdtyIndex, creator: &IdtyIndex) {
-        // identity just has been created, a cert must be added
         <pallet_certification::Pallet<T>>::do_add_cert_checked(*creator, *idty_index, true);
     }
 }
 
+/// Implementing identity removal event handling for the pallet.
 impl<T: Config> pallet_identity::traits::OnRemoveIdty<T> for Pallet<T> {
-    // Remove membership and certificates.
+    /// This implementation removes both membership and certificates associated with the identity.
     fn on_removed(idty_index: &IdtyIndex) -> Weight {
         let mut weight = Self::on_revoked(idty_index);
         weight = weight.saturating_add(
@@ -238,7 +255,7 @@ impl<T: Config> pallet_identity::traits::OnRemoveIdty<T> for Pallet<T> {
         weight
     }
 
-    // Remove membership only.
+    /// This implementation removes membership only.
     fn on_revoked(idty_index: &IdtyIndex) -> Weight {
         let mut weight = Weight::zero();
         weight = weight.saturating_add(<pallet_membership::Pallet<T>>::do_remove_membership(
@@ -249,9 +266,10 @@ impl<T: Config> pallet_identity::traits::OnRemoveIdty<T> for Pallet<T> {
     }
 }
 
-// implement certification event handlers
-// new cert handler
+/// Implementing the certification event handler for the pallet.
 impl<T: Config> pallet_certification::traits::OnNewcert<IdtyIndex> for Pallet<T> {
+    /// This implementation checks if the receiver has received enough certificates to be able to issue certificates,
+    /// and applies the first issuable if the condition is met.
     fn on_new_cert(
         _issuer: IdtyIndex,
         _issuer_issued_count: u32,
@@ -264,8 +282,10 @@ impl<T: Config> pallet_certification::traits::OnNewcert<IdtyIndex> for Pallet<T>
     }
 }
 
-// remove cert handler
+/// Implementing the certification removal event handler for the pallet.
 impl<T: Config> pallet_certification::traits::OnRemovedCert<IdtyIndex> for Pallet<T> {
+    /// This implementation checks if the receiver has received fewer certificates than required for membership,
+    /// and if so, and the receiver is a member, it expires the receiver's membership.
     fn on_removed_cert(
         _issuer: IdtyIndex,
         _issuer_issued_count: u32,
@@ -276,7 +296,7 @@ impl<T: Config> pallet_certification::traits::OnRemovedCert<IdtyIndex> for Palle
         if receiver_received_count < T::MinCertForMembership::get()
             && pallet_membership::Pallet::<T>::is_member(&receiver)
         {
-            // expire receiver membership
+            // Expire receiver membership
             <pallet_membership::Pallet<T>>::do_remove_membership(
                 receiver,
                 MembershipRemovalReason::NotEnoughCerts,
@@ -285,10 +305,15 @@ impl<T: Config> pallet_certification::traits::OnRemovedCert<IdtyIndex> for Palle
     }
 }
 
-/// valid distance status handler
+/// Implementing the valid distance status event handler for the pallet.
 impl<T: Config + pallet_distance::Config> pallet_distance::traits::OnValidDistanceStatus<T>
     for Pallet<T>
 {
+    /// This implementation handles different scenarios based on the identity's status:
+    ///
+    /// - For `Unconfirmed` or `Revoked` identities, no action is taken.
+    /// - For `Unvalidated` or `NotMember` identities, an attempt is made to add membership.
+    /// - For `Member` identities, an attempt is made to renew membership.
     fn on_valid_distance_status(idty_index: IdtyIndex) {
         if let Some(identity) = pallet_identity::Identities::<T>::get(idty_index) {
             match identity.status {
@@ -339,15 +364,19 @@ impl<T: Config + pallet_distance::Config> pallet_distance::traits::OnValidDistan
     }
 }
 
-/// distance evaluation request allowed check
+/// Implementing the request distance evaluation check for the pallet.
 impl<T: Config + pallet_distance::Config> pallet_distance::traits::CheckRequestDistanceEvaluation<T>
     for Pallet<T>
 {
+    /// This implementation performs the following checks:
+    ///
+    /// - Membership renewal anti-spam check: Ensures that membership renewal requests respect the anti-spam period.
+    /// - Certificate count check: Ensures that the identity has a sufficient number of certificates.
     fn check_request_distance_evaluation(idty_index: IdtyIndex) -> Result<(), DispatchError> {
-        // check membership renewal antispam
+        // Check membership renewal anti-spam
         let maybe_membership_data = pallet_membership::Pallet::<T>::membership(idty_index);
         if let Some(membership_data) = maybe_membership_data {
-            // if membership data exists, this is for a renewal, apply antispam
+            // If membership data exists, this is for a renewal, apply anti-spam
             ensure!(
                 // current_block > expiration block - membership period + renewal period
                 membership_data.expire_on
@@ -357,13 +386,13 @@ impl<T: Config + pallet_distance::Config> pallet_distance::traits::CheckRequestD
                 Error::<T>::MembershipRenewalPeriodNotRespected
             );
         };
-        // check cert count
+        // Check certificate count
         check_cert_count::<T>(idty_index)?;
         Ok(())
     }
 }
 
-/// check certification count
+/// Checks the certificate count for an identity.
 fn check_cert_count<T: Config>(idty_index: IdtyIndex) -> Result<(), DispatchError> {
     let idty_cert_meta = pallet_certification::Pallet::<T>::idty_cert_meta(idty_index);
     ensure!(
diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs
index 65679ca7d..1a7b30093 100644
--- a/pallets/identity/src/traits.rs
+++ b/pallets/identity/src/traits.rs
@@ -19,7 +19,9 @@ use frame_support::pallet_prelude::*;
 use frame_support::weights::Weight;
 use impl_trait_for_tuples::impl_for_tuples;
 
+/// A trait defining operations for checking if identity-related calls are allowed.
 pub trait CheckIdtyCallAllowed<T: Config> {
+    /// Checks if creating an identity is allowed.
     fn check_create_identity(creator: T::IdtyIndex) -> Result<(), DispatchError>;
 }
 
@@ -31,16 +33,25 @@ impl<T: Config> CheckIdtyCallAllowed<T> for Tuple {
     }
 }
 
+/// A trait defining operations for validating identity names.
 pub trait IdtyNameValidator {
+    /// Validates an identity name.
     fn validate(idty_name: &IdtyName) -> bool;
 }
 
+/// A trait defining behavior for handling new identities creation.
 pub trait OnNewIdty<T: Config> {
+    /// Called when a new identity is created.
     fn on_created(idty_index: &T::IdtyIndex, creator: &T::IdtyIndex);
 }
 
+/// A trait defining behavior for handling removed identities.
+/// As the weight accounting can be complicated it should be done
+/// at the handler level.
 pub trait OnRemoveIdty<T: Config> {
+    /// Called when an identity is removed.
     fn on_removed(idty_index: &T::IdtyIndex) -> Weight;
+    /// Called when an identity is revoked.
     fn on_revoked(idty_index: &T::IdtyIndex) -> Weight;
 }
 
@@ -68,8 +79,9 @@ impl<T: Config> OnRemoveIdty<T> for Tuple {
     }
 }
 
-/// trait used to link an account to an identity
+/// A trait defining operations for linking identities to accounts.
 pub trait LinkIdty<AccountId, IdtyIndex> {
+    /// Links an identity to an account.
     fn link_identity(account_id: &AccountId, idty_index: IdtyIndex) -> Result<(), DispatchError>;
 }
 impl<AccountId, IdtyIndex> LinkIdty<AccountId, IdtyIndex> for () {
diff --git a/pallets/quota/src/lib.rs b/pallets/quota/src/lib.rs
index 85d0a35e2..7c25f1fe7 100644
--- a/pallets/quota/src/lib.rs
+++ b/pallets/quota/src/lib.rs
@@ -316,8 +316,9 @@ pub mod pallet {
     }
 }
 
-// implement quota traits
+/// Implementing the refund fee trait for the pallet.
 impl<T: Config> RefundFee<T> for Pallet<T> {
+    /// This implementation checks if the identity is eligible for a refund and queues the refund if so.
     fn request_refund(account: T::AccountId, identity: IdtyId<T>, amount: BalanceOf<T>) {
         if is_eligible_for_refund::<T>(identity) {
             Self::queue_refund(Refund {
@@ -329,16 +330,18 @@ impl<T: Config> RefundFee<T> for Pallet<T> {
     }
 }
 
-/// tells whether an identity is eligible for refund
+/// Checks if an identity is eligible for a refund.
+///
+/// This function returns `true` for all identities, regardless of their status.
+/// If the identity has no quotas or has been deleted, the refund request is still queued,
+/// but when handled, no refund will be issued (and `NoQuotaForIdty` may be raised).
 fn is_eligible_for_refund<T: pallet_identity::Config>(_identity: IdtyId<T>) -> bool {
-    // all identities are eligible for refund, no matter their status
-    // if the identity has no quotas or has been deleted, the refund request is still queued
-    // but when handeled, no refund will be issued (and `NoQuotaForIdty` may be raised)
     true
 }
 
-// implement identity event handler
+/// Implementing the on new identity event handler for the pallet.
 impl<T: Config> pallet_identity::traits::OnNewIdty<T> for Pallet<T> {
+    /// This implementation initializes the identity quota for the newly created identity.
     fn on_created(idty_index: &IdtyId<T>, _creator: &T::IdtyIndex) {
         IdtyQuota::<T>::insert(
             idty_index,
@@ -350,7 +353,9 @@ impl<T: Config> pallet_identity::traits::OnNewIdty<T> for Pallet<T> {
     }
 }
 
+/// Implementing the on remove identity event handler for the pallet.
 impl<T: Config> pallet_identity::traits::OnRemoveIdty<T> for Pallet<T> {
+    /// This implementation removes the identity quota associated with the removed identity.
     fn on_removed(idty_id: &IdtyId<T>) -> Weight {
         let mut weight = Weight::zero();
         let mut add_db_reads_writes = |reads, writes| {
@@ -362,6 +367,7 @@ impl<T: Config> pallet_identity::traits::OnRemoveIdty<T> for Pallet<T> {
         weight
     }
 
+    /// This implementation removes the identity quota associated with the removed identity.
     fn on_revoked(idty_id: &IdtyId<T>) -> Weight {
         Self::on_removed(idty_id)
     }
diff --git a/primitives/membership/src/traits.rs b/primitives/membership/src/traits.rs
index 2e5a30930..7eef2c1bf 100644
--- a/primitives/membership/src/traits.rs
+++ b/primitives/membership/src/traits.rs
@@ -38,14 +38,18 @@ impl<IdtyId> CheckMembershipOpAllowed<IdtyId> for () {
 pub trait OnNewMembership<IdtyId> {
     /// Called when a new membership is created.
     fn on_created(idty_index: &IdtyId);
+    /// Called when a membership is renewed.
     fn on_renewed(idty_index: &IdtyId);
 }
 
-/// Called when an existing membership is renewed.
+/// A trait defining operations for handling the removal of memberships.
 pub trait OnRemoveMembership<IdtyId> {
+    /// Called when a membership is removed.
     fn on_removed(idty_index: &IdtyId) -> Weight;
 }
 
+/// A trait defining an operation to retrieve the count of members.
 pub trait MembersCount {
+    /// The count of members.
     fn members_count() -> u32;
 }
-- 
GitLab