From 55d5123737b0f1cdab6ca99e5454f85c0b694565 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Moreau?= <cem.moreau@gmail.com>
Date: Mon, 22 Jan 2024 15:18:18 +0100
Subject: [PATCH] Improvements for smith-members (nodes/rust/duniter-v2s!228)

* refac(#173): harmonize InvitationSent

* refac(#173): update metadata.scale + docs

* refac(#173): harmonize *MembershipAdded and *MembershipRemoved

* refac(#173): harmonize *CertAdded and *CertRemoved

* feat(#173): docstring for calls

* feat(#173): add SmithCertificationRemoved event

* feat(#173): rename CertificationReceived to SmithCertificationAdded
---
 docs/api/runtime-calls.md                 |   6 +-
 docs/api/runtime-events.md                |  31 +++++++---
 pallets/smith-members/src/benchmarking.rs |  10 ++--
 pallets/smith-members/src/lib.rs          |  54 ++++++++++-------
 pallets/smith-members/src/tests.rs        |  70 +++++++++++++++-------
 resources/metadata.scale                  | Bin 128457 -> 128711 bytes
 runtime/gdev/tests/integration_tests.rs   |  14 ++++-
 7 files changed, 123 insertions(+), 62 deletions(-)

diff --git a/docs/api/runtime-calls.md b/docs/api/runtime-calls.md
index 3c96e8358..971f556b4 100644
--- a/docs/api/runtime-calls.md
+++ b/docs/api/runtime-calls.md
@@ -359,7 +359,7 @@ receiver: T::IdtyIndex
 </details>
 
 
-
+Invite a WoT member to try becoming a Smith
 
 #### accept_invitation - 1
 
@@ -372,7 +372,7 @@ Taking 0.0085 % of a block.
 </details>
 
 
-
+Accept an invitation (must have been invited first)
 
 #### certify_smith - 2
 
@@ -386,7 +386,7 @@ receiver: T::IdtyIndex
 </details>
 
 
-
+Certify an invited smith which can lead the certified to become a Smith
 
 ### AuthorityMembers - 11
 
diff --git a/docs/api/runtime-events.md b/docs/api/runtime-events.md
index 566e73839..a3a532d8a 100644
--- a/docs/api/runtime-events.md
+++ b/docs/api/runtime-events.md
@@ -1,6 +1,6 @@
 # Runtime events
 
-There are **128** events from **35** pallets.
+There are **129** events from **35** pallets.
 
 <ul>
 <li>System - 0
@@ -649,12 +649,12 @@ no args
 <li>
 <details>
 <summary>
-<code>InvitationSent(idty_index, invited_by)</code> - 0</summary>
+<code>InvitationSent(receiver, issuer)</code> - 0</summary>
 An identity is being inivited to become a smith.
 
 ```rust
-idty_index: T::IdtyIndex
-invited_by: T::IdtyIndex
+receiver: T::IdtyIndex
+issuer: T::IdtyIndex
 ```
 
 </details>
@@ -674,12 +674,25 @@ idty_index: T::IdtyIndex
 <li>
 <details>
 <summary>
-<code>CertificationReceived(idty_index, issued_by)</code> - 2</summary>
+<code>SmithCertAdded(receiver, issuer)</code> - 2</summary>
 Certification received
 
 ```rust
-idty_index: T::IdtyIndex
-issued_by: T::IdtyIndex
+receiver: T::IdtyIndex
+issuer: T::IdtyIndex
+```
+
+</details>
+</li>
+<li>
+<details>
+<summary>
+<code>SmithCertRemoved(receiver, issuer)</code> - 3</summary>
+Certification lost
+
+```rust
+receiver: T::IdtyIndex
+issuer: T::IdtyIndex
 ```
 
 </details>
@@ -687,7 +700,7 @@ issued_by: T::IdtyIndex
 <li>
 <details>
 <summary>
-<code>PromotedToSmith(idty_index)</code> - 3</summary>
+<code>SmithMembershipAdded(idty_index)</code> - 4</summary>
 A smith gathered enough certifications to become an authority (can call `go_online()`).
 
 ```rust
@@ -699,7 +712,7 @@ idty_index: T::IdtyIndex
 <li>
 <details>
 <summary>
-<code>SmithExcluded(idty_index)</code> - 4</summary>
+<code>SmithMembershipRemoved(idty_index)</code> - 5</summary>
 A smith has been removed from the smiths set.
 
 ```rust
diff --git a/pallets/smith-members/src/benchmarking.rs b/pallets/smith-members/src/benchmarking.rs
index 071d73905..62e0b8ddc 100644
--- a/pallets/smith-members/src/benchmarking.rs
+++ b/pallets/smith-members/src/benchmarking.rs
@@ -45,8 +45,8 @@ benchmarks! {
     }: _<T::RuntimeOrigin>(caller_origin, receiver)
     verify {
         assert_has_event::<T>(Event::<T>::InvitationSent{
-            idty_index: receiver,
-            invited_by: issuer,
+            receiver,
+            issuer,
         }.into());
     }
     accept_invitation {
@@ -85,9 +85,9 @@ benchmarks! {
         let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into();
     }: _<T::RuntimeOrigin>(caller_origin, receiver)
     verify {
-        assert_has_event::<T>(Event::<T>::CertificationReceived{
-            idty_index: receiver,
-            issued_by: issuer,
+        assert_has_event::<T>(Event::<T>::SmithCertAdded{
+            receiver,
+            issuer,
         }.into());
     }
 
diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 8d4df3fa6..a20e0bd04 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -130,20 +130,25 @@ pub mod pallet {
     pub enum Event<T: Config> {
         /// An identity is being inivited to become a smith.
         InvitationSent {
-            idty_index: T::IdtyIndex,
-            invited_by: T::IdtyIndex,
+            receiver: T::IdtyIndex,
+            issuer: T::IdtyIndex,
         },
         /// The invitation has been accepted.
         InvitationAccepted { idty_index: T::IdtyIndex },
         /// Certification received
-        CertificationReceived {
-            idty_index: T::IdtyIndex,
-            issued_by: T::IdtyIndex,
+        SmithCertAdded {
+            receiver: T::IdtyIndex,
+            issuer: T::IdtyIndex,
+        },
+        /// Certification lost
+        SmithCertRemoved {
+            receiver: T::IdtyIndex,
+            issuer: T::IdtyIndex,
         },
         /// A smith gathered enough certifications to become an authority (can call `go_online()`).
-        PromotedToSmith { idty_index: T::IdtyIndex },
+        SmithMembershipAdded { idty_index: T::IdtyIndex },
         /// A smith has been removed from the smiths set.
-        SmithExcluded { idty_index: T::IdtyIndex },
+        SmithMembershipRemoved { idty_index: T::IdtyIndex },
     }
 
     #[pallet::genesis_config]
@@ -275,6 +280,7 @@ pub mod pallet {
 
     #[pallet::call]
     impl<T: Config> Pallet<T> {
+        /// Invite a WoT member to try becoming a Smith
         #[pallet::call_index(0)]
         #[pallet::weight(T::WeightInfo::invite_smith())]
         pub fn invite_smith(
@@ -289,6 +295,7 @@ pub mod pallet {
             Ok(().into())
         }
 
+        /// Accept an invitation (must have been invited first)
         #[pallet::call_index(1)]
         #[pallet::weight(T::WeightInfo::accept_invitation())]
         pub fn accept_invitation(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
@@ -300,6 +307,7 @@ pub mod pallet {
             Ok(().into())
         }
 
+        /// Certify an invited smith which can lead the certified to become a Smith
         #[pallet::call_index(2)]
         #[pallet::weight(T::WeightInfo::certify_smith())]
         pub fn certify_smith(
@@ -352,10 +360,7 @@ impl<T: Config> Pallet<T> {
         existing.received_certs = vec![];
         Smiths::<T>::insert(receiver, existing);
         ExpiresOn::<T>::append(new_expires_on, receiver);
-        Self::deposit_event(Event::<T>::InvitationSent {
-            idty_index: receiver,
-            invited_by: issuer,
-        });
+        Self::deposit_event(Event::<T>::InvitationSent { receiver, issuer });
     }
 
     fn check_accept_invitation(receiver: T::IdtyIndex) -> DispatchResultWithPostInfo {
@@ -445,12 +450,9 @@ impl<T: Config> Pallet<T> {
                 let new_expires_on =
                     CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get();
                 smith_meta.expires_on = Some(new_expires_on);
-                Self::deposit_event(Event::<T>::CertificationReceived {
-                    idty_index: receiver,
-                    issued_by: issuer,
-                });
+                Self::deposit_event(Event::<T>::SmithCertAdded { receiver, issuer });
                 if smith_meta.status == SmithStatus::Smith {
-                    Self::deposit_event(Event::<T>::PromotedToSmith {
+                    Self::deposit_event(Event::<T>::SmithMembershipAdded {
                         idty_index: receiver,
                     });
                 }
@@ -479,9 +481,9 @@ impl<T: Config> Pallet<T> {
         }
     }
 
-    fn _do_exclude_smith(idty_index: T::IdtyIndex, reason: SmithRemovalReason) {
+    fn _do_exclude_smith(receiver: T::IdtyIndex, reason: SmithRemovalReason) {
         let mut lost_certs = vec![];
-        Smiths::<T>::mutate(idty_index, |maybe_smith_meta| {
+        Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
             if let Some(smith_meta) = maybe_smith_meta {
                 smith_meta.expires_on = None;
                 smith_meta.status = SmithStatus::Excluded;
@@ -493,18 +495,24 @@ impl<T: Config> Pallet<T> {
             }
         });
         // We remove the lost certs from their issuer's stock
-        for lost_cert in lost_certs {
-            Smiths::<T>::mutate(lost_cert, |maybe_smith_meta| {
+        for lost_cert_issuer in lost_certs {
+            Smiths::<T>::mutate(lost_cert_issuer, |maybe_smith_meta| {
                 if let Some(smith_meta) = maybe_smith_meta {
-                    if let Ok(index) = smith_meta.issued_certs.binary_search(&idty_index) {
+                    if let Ok(index) = smith_meta.issued_certs.binary_search(&receiver) {
                         smith_meta.issued_certs.remove(index);
+                        Self::deposit_event(Event::<T>::SmithCertRemoved {
+                            receiver,
+                            issuer: lost_cert_issuer,
+                        });
                     }
                 }
             });
         }
         // Deletion done: notify (authority-members) for cascading
-        T::OnSmithDelete::on_smith_delete(idty_index, reason);
-        Self::deposit_event(Event::<T>::SmithExcluded { idty_index });
+        T::OnSmithDelete::on_smith_delete(receiver, reason);
+        Self::deposit_event(Event::<T>::SmithMembershipRemoved {
+            idty_index: receiver,
+        });
     }
 
     pub fn on_smith_goes_online(idty_index: T::IdtyIndex) {
diff --git a/pallets/smith-members/src/tests.rs b/pallets/smith-members/src/tests.rs
index a843a17a9..a6b9f0715 100644
--- a/pallets/smith-members/src/tests.rs
+++ b/pallets/smith-members/src/tests.rs
@@ -46,8 +46,8 @@ fn process_to_become_a_smith_and_lose_it() {
         // Try to invite
         assert_ok!(Pallet::<Runtime>::invite_smith(RuntimeOrigin::signed(1), 5));
         System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::InvitationSent {
-            idty_index: 5,
-            invited_by: 1,
+            receiver: 5,
+            issuer: 1,
         }));
         // Accept invitation
         assert_ok!(Pallet::<Runtime>::accept_invitation(RuntimeOrigin::signed(
@@ -71,12 +71,10 @@ fn process_to_become_a_smith_and_lose_it() {
             RuntimeOrigin::signed(1),
             5
         ));
-        System::assert_has_event(RuntimeEvent::Smith(
-            Event::<Runtime>::CertificationReceived {
-                idty_index: 5,
-                issued_by: 1,
-            },
-        ));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertAdded {
+            receiver: 5,
+            issuer: 1,
+        }));
         assert_eq!(
             Smiths::<Runtime>::get(5).unwrap(),
             SmithMeta {
@@ -91,15 +89,13 @@ fn process_to_become_a_smith_and_lose_it() {
             RuntimeOrigin::signed(2),
             5
         ));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertAdded {
+            receiver: 5,
+            issuer: 1,
+        }));
         System::assert_has_event(RuntimeEvent::Smith(
-            Event::<Runtime>::CertificationReceived {
-                idty_index: 5,
-                issued_by: 1,
-            },
+            Event::<Runtime>::SmithMembershipAdded { idty_index: 5 },
         ));
-        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::PromotedToSmith {
-            idty_index: 5,
-        }));
         assert_eq!(
             Smiths::<Runtime>::get(5).unwrap(),
             SmithMeta {
@@ -120,14 +116,46 @@ fn process_to_become_a_smith_and_lose_it() {
         assert!(Smiths::<Runtime>::get(5).is_some());
         // On session 5 no more smiths because of lack of activity
         Pallet::<Runtime>::on_new_session(5);
-        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithExcluded {
-            idty_index: 1,
+        System::assert_has_event(RuntimeEvent::Smith(
+            Event::<Runtime>::SmithMembershipRemoved { idty_index: 1 },
+        ));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 1,
+            issuer: 2,
         }));
-        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithExcluded {
-            idty_index: 2,
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 1,
+            issuer: 3,
         }));
-        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithExcluded {
-            idty_index: 5,
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 1,
+            issuer: 4,
+        }));
+        System::assert_has_event(RuntimeEvent::Smith(
+            Event::<Runtime>::SmithMembershipRemoved { idty_index: 2 },
+        ));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 2,
+            issuer: 3,
+        }));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 2,
+            issuer: 4,
+        }));
+        System::assert_has_event(RuntimeEvent::Smith(
+            Event::<Runtime>::SmithMembershipRemoved { idty_index: 5 },
+        ));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 1,
+            issuer: 3,
+        }));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 5,
+            issuer: 1,
+        }));
+        System::assert_has_event(RuntimeEvent::Smith(Event::<Runtime>::SmithCertRemoved {
+            receiver: 5,
+            issuer: 2,
         }));
         assert_eq!(
             Smiths::<Runtime>::get(1),
diff --git a/resources/metadata.scale b/resources/metadata.scale
index 820374cd2073ee49d2ba67359255742635e037a9..ec4593b3867538d339e681f0f8c0013569459624 100644
GIT binary patch
delta 399
zcmX^4n*I1&_6>b%f)W;<d1aX;i6xo&dBLf9B^;CQ&6Jufa7|+Jb+vW@i{RYMk__k6
zq7uiHl++Z2<QjEJAqTi*P-<>|8IrUHGm8gEQ)XIbGT3B=oc!XF0I+)B)ZC=hqT-Cq
z0-%wT3+?3@Stfs0j}nbQlLeazmS^2usnJka&$0&W(o}^+h4B0kg<P;X3MKgpB}J7A
zNvX;CxtV$CKuNGUE{Vy>sRbqRnQ+fBFtVI+1Pdx8<|!b=6*O{7i%S$T63bG7rldlp
zQd1PtGK-2!G)<Dh{-}(fEHO*bi$#_Z;;%}$AwZSIpg>Y6&&W*9P)G(Eo|BrGqEM2N
issPcO2~+}fCfJD}*Ml86dE*bC=C)be+h#F3M*{$4F_r27

delta 183
zcmX^9mi^>w_6>b%f+7~4d1aX;i6xo&dBLf9B^;9-?Pa+&GC_i=De*~_lMkv*7Y}hx
zEh@=O%S;BV3rbB+%`8hz0jZVa%q%W0g{bUO*A%b`D9X>x2Wkz;56;aj$(XFDA<M`-
z*<K@x*90u!T9KSnnvy#Cpqm^c%jR<$4TWM1E{Vy>sRbqRU|YdfFfcMqJ~B%SNZL2}
OG`Gy&-ZGofIT`?$;X^_I

diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index cfc3cb05d..82e8bf5ed 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -1540,7 +1540,19 @@ fn test_smith_member_can_revoke_its_idty() {
         ));
         // smith membership should be removed as well
         System::assert_has_event(RuntimeEvent::SmithMembers(
-            pallet_smith_members::Event::SmithExcluded { idty_index: 3 },
+            pallet_smith_members::Event::SmithMembershipRemoved { idty_index: 3 },
+        ));
+        System::assert_has_event(RuntimeEvent::SmithMembers(
+            pallet_smith_members::Event::SmithCertRemoved {
+                receiver: 3,
+                issuer: 1,
+            },
+        ));
+        System::assert_has_event(RuntimeEvent::SmithMembers(
+            pallet_smith_members::Event::SmithCertRemoved {
+                receiver: 3,
+                issuer: 2,
+            },
         ));
         // Now Charlie is going out
         assert!(pallet_authority_members::OutgoingAuthorities::<Runtime>::get().contains(&3));
-- 
GitLab