feat(smith-members): remove CertsByReceiver (all in Smiths)

pallets/smith-members/src/lib.rs

@@ -173,7 +173,6 @@ pub mod pallet {
                 // Write CertsByReceiver
                 issuers_.sort();
                 let issuers_count = issuers_.len();
-                CertsByReceiver::<T>::insert(receiver, issuers_);
                 let smith_status = if issuers_count as u32 >= T::MinCertForMembership::get() {
                     SmithStatus::Smith
                 } else {
@@ -186,6 +185,7 @@ pub mod pallet {
                         expires_on: Some(
                             CurrentSession::<T>::get() + T::InactivityMaxDuration::get(),
                         ),
+                        received_certs: issuers_,
                     },
                 );
                 ExpiresOn::<T>::append(
@@ -196,14 +196,10 @@ pub mod pallet {
         }
     }
 
-    /// maps identity index to smith value
-    #[pallet::storage]
-    pub type CertsByReceiver<T: Config> =
-        StorageMap<_, Twox64Concat, T::IdtyIndex, Vec<T::IdtyIndex>, OptionQuery>;
-
     /// maps identity index to smith status
     #[pallet::storage]
-    pub type Smiths<T: Config> = StorageMap<_, Twox64Concat, T::IdtyIndex, SmithMeta, OptionQuery>;
+    pub type Smiths<T: Config> =
+        StorageMap<_, Twox64Concat, T::IdtyIndex, SmithMeta<T::IdtyIndex>, OptionQuery>;
 
     /// maps session index to possible smith removals
     #[pallet::storage]
@@ -242,6 +238,8 @@ pub mod pallet {
         NotAMember,
     }
 
+    // TODO: refactor with check_* and do_* functions
+
     #[pallet::call]
     impl<T: Config> Pallet<T> {
         #[pallet::call_index(0)]
@@ -270,6 +268,7 @@ pub mod pallet {
                 SmithMeta {
                     status: SmithStatus::Invited,
                     expires_on: Some(new_expires_on),
+                    received_certs: vec![],
                 },
             );
             ExpiresOn::<T>::append(new_expires_on, receiver);
@@ -322,20 +321,22 @@ pub mod pallet {
                 Error::<T>::ReceveirMustAcceptInvitation
             );
 
-            CertsByReceiver::<T>::mutate(receiver, |certs| {
-                let certs = certs.get_or_insert(vec![]);
-                certs.push(issuer);
             Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
-                    let maybe_smith_meta =
-                        maybe_smith_meta.as_mut().expect("status checked earlier");
+                let maybe_smith_meta = maybe_smith_meta.as_mut().expect("status checked earlier");
+                maybe_smith_meta.received_certs.push(issuer);
+                maybe_smith_meta.received_certs.sort();
                 // TODO: "as u32" allowed?
-                    maybe_smith_meta.status =
-                        if certs.len() as u32 >= T::MinCertForMembership::get() {
+                maybe_smith_meta.status = if maybe_smith_meta.received_certs.len() as u32
+                    >= T::MinCertForMembership::get()
+                {
                     SmithStatus::Smith
                 } else {
                     SmithStatus::Pending
                 };
-                });
+                // expiry postponed
+                let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get();
+                maybe_smith_meta.expires_on = Some(new_expires_on);
+                // TODO: unschedule old expiry
             });
 
             Ok(().into())
@@ -405,11 +406,11 @@ impl<T: Config> Pallet<T> {
 
 impl<T: Config> Pallet<T> {
     fn do_is_member(idty_id: &T::IdtyIndex) -> bool {
-        let Some(certifications) = CertsByReceiver::<T>::get(idty_id) else {
+        let Some(smith) = Smiths::<T>::get(idty_id) else {
             return false;
         };
         let mut count = 0u32;
-        for _ in certifications {
+        for _ in smith.received_certs {
             count += 1;
         }
         return count >= T::MinCertForMembership::get();

pallets/smith-members/src/tests.rs

@@ -42,7 +42,6 @@ fn process_to_become_a_smith_and_lose_it() {
     })
     .execute_with(|| {
         // State before
-        assert_eq!(CertsByReceiver::<Runtime>::get(5), None);
         assert_eq!(Smiths::<Runtime>::get(5), None);
         // Try to invite
         assert_ok!(Pallet::<Runtime>::invite_smith(RuntimeOrigin::signed(1), 5));
@@ -51,30 +50,39 @@ fn process_to_become_a_smith_and_lose_it() {
             5
         )));
         // State after
-        assert_eq!(CertsByReceiver::<Runtime>::get(5), None);
         assert_eq!(
-            Smiths::<Runtime>::get(5).unwrap().status,
-            SmithStatus::Pending
+            Smiths::<Runtime>::get(5).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Pending,
+                expires_on: Some(5),
+                received_certs: vec![],
+            }
         );
         // Then certification 1/2
         assert_ok!(Pallet::<Runtime>::certify_smith(
             RuntimeOrigin::signed(1),
             5
         ));
-        assert_eq!(CertsByReceiver::<Runtime>::get(5).unwrap().len(), 1);
         assert_eq!(
-            Smiths::<Runtime>::get(5).unwrap().status,
-            SmithStatus::Pending
+            Smiths::<Runtime>::get(5).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Pending,
+                expires_on: Some(5),
+                received_certs: vec![1],
+            }
         );
         // Then certification 2/2
         assert_ok!(Pallet::<Runtime>::certify_smith(
             RuntimeOrigin::signed(2),
             5
         ));
-        assert_eq!(CertsByReceiver::<Runtime>::get(5).unwrap().len(), 2);
         assert_eq!(
-            Smiths::<Runtime>::get(5).unwrap().status,
-            SmithStatus::Smith
+            Smiths::<Runtime>::get(5).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Smith,
+                expires_on: Some(5),
+                received_certs: vec![1, 2],
+            }
         );
 
         // On session 4 everything if fine
@@ -84,9 +92,9 @@ fn process_to_become_a_smith_and_lose_it() {
         assert_eq!(Smiths::<Runtime>::get(5).is_some(), true);
         // On session 5 no more smiths because of lack of activity
         Pallet::<Runtime>::on_new_session(5);
-        assert_eq!(Smiths::<Runtime>::get(1), None);
-        assert_eq!(Smiths::<Runtime>::get(2), None);
-        assert_eq!(Smiths::<Runtime>::get(5), None);
+        assert_eq!(Smiths::<Runtime>::get(1), None); // TODO: noautoremove (remaining certs)
+        assert_eq!(Smiths::<Runtime>::get(2), None); // TODO: noautoremove (remaining certs)
+        assert_eq!(Smiths::<Runtime>::get(5), None); // TODO: noautoremove (remaining certs)
     });
 }
 
@@ -114,27 +122,38 @@ fn should_have_checks_on_certify() {
     })
     .execute_with(|| {
         // Initially
-        assert_eq!(CertsByReceiver::<Runtime>::iter().count(), 4);
         assert_eq!(
-            Smiths::<Runtime>::get(1).unwrap().status,
-            SmithStatus::Smith
+            Smiths::<Runtime>::get(1).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Smith,
+                expires_on: Some(5),
+                received_certs: vec![2, 3, 4],
+            }
         );
         assert_eq!(
-            Smiths::<Runtime>::get(2).unwrap().status,
-            SmithStatus::Smith
+            Smiths::<Runtime>::get(2).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Smith,
+                expires_on: Some(5),
+                received_certs: vec![3, 4],
+            }
         );
         assert_eq!(
-            Smiths::<Runtime>::get(3).unwrap().status,
-            SmithStatus::Pending
+            Smiths::<Runtime>::get(3).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Pending,
+                expires_on: Some(5),
+                received_certs: vec![4],
+            }
         );
         assert_eq!(
-            Smiths::<Runtime>::get(4).unwrap().status,
-            SmithStatus::Smith
+            Smiths::<Runtime>::get(4).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Smith,
+                expires_on: Some(5),
+                received_certs: vec![1, 2],
+            }
         );
-        assert_eq!(CertsByReceiver::<Runtime>::get(1).unwrap().len(), 3);
-        assert_eq!(CertsByReceiver::<Runtime>::get(2).unwrap().len(), 2);
-        assert_eq!(CertsByReceiver::<Runtime>::get(3).unwrap().len(), 1);
-        assert_eq!(CertsByReceiver::<Runtime>::get(4).unwrap().len(), 2);
 
         // Tries all possible errors
         assert_err!(
@@ -155,10 +174,13 @@ fn should_have_checks_on_certify() {
         );
 
         // #3: state before
-        assert_eq!(CertsByReceiver::<Runtime>::get(3).unwrap().len(), 1);
         assert_eq!(
-            Smiths::<Runtime>::get(3).unwrap().status,
-            SmithStatus::Pending
+            Smiths::<Runtime>::get(3).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Pending,
+                expires_on: Some(5),
+                received_certs: vec![4],
+            }
         );
         // Try to certify #3
         assert_ok!(Pallet::<Runtime>::certify_smith(
@@ -166,10 +188,13 @@ fn should_have_checks_on_certify() {
             3
         ));
         // #3: state after
-        assert_eq!(CertsByReceiver::<Runtime>::get(3).unwrap().len(), 2);
         assert_eq!(
-            Smiths::<Runtime>::get(3).unwrap().status,
-            SmithStatus::Smith
+            Smiths::<Runtime>::get(3).unwrap(),
+            SmithMeta {
+                status: SmithStatus::Smith,
+                expires_on: Some(5),
+                received_certs: vec![1, 4],
+            }
         );
     });
 }
@@ -184,9 +209,8 @@ fn smith_activity_postpones_expiration() {
                 4 => None,
             ],
             2 => btreemap![
+                3 => None,
                 4 => None,
-                4 => None,
-                5 => None,
             ]
         ],
     })
@@ -206,7 +230,8 @@ fn smith_activity_postpones_expiration() {
             Smiths::<Runtime>::get(2),
             Some(SmithMeta {
                 status: SmithStatus::Smith,
-                expires_on: None
+                expires_on: None,
+                received_certs: vec![3, 4],
             })
         );
 
@@ -218,7 +243,8 @@ fn smith_activity_postpones_expiration() {
             Smiths::<Runtime>::get(2),
             Some(SmithMeta {
                 status: SmithStatus::Smith,
-                expires_on: Some(11)
+                expires_on: Some(11),
+                received_certs: vec![3, 4],
             })
         );
         // Still not expired on session 10
@@ -228,7 +254,8 @@ fn smith_activity_postpones_expiration() {
             Smiths::<Runtime>::get(2),
             Some(SmithMeta {
                 status: SmithStatus::Smith,
-                expires_on: Some(11)
+                expires_on: Some(11),
+                received_certs: vec![3, 4],
             })
         );
         // Finally expired on session 11

pallets/smith-members/src/types.rs

@@ -24,14 +24,17 @@ use pallet_authority_members::SessionIndex;
 use scale_info::TypeInfo;
 
 /// certification metadata attached to an identity
-#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo)]
-pub struct SmithMeta {
+#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)]
+pub struct SmithMeta<IdtyIndex> {
     /// current status of the smith
     pub status: SmithStatus,
     /// the session at which the smith will expire (for lack of validation activity)
     pub expires_on: Option<SessionIndex>,
+    /// the recertifications received from other smiths
+    pub received_certs: Vec<IdtyIndex>,
 }
 
+// TODO: check what is really used
 /// certification metadata attached to an identity
 #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo)]
 pub struct SmithCertMeta<BlockNumber: Default> {