refac certification creation checks

pallets/certification/src/lib.rs

@@ -283,38 +283,8 @@ pub mod pallet {
         ) -> DispatchResultWithPostInfo {
             let who = ensure_signed(origin)?;
 
-            // Forbid self cert
-            ensure!(issuer != receiver, Error::<T, I>::CannotCertifySelf);
-
-            // Verify caller ownership
-            let issuer_owner_key =
-                T::OwnerKeyOf::convert(issuer).ok_or(Error::<T, I>::IssuerNotFound)?;
-            ensure!(issuer_owner_key == who, DispatchError::BadOrigin);
-
-            // Verify compatibility with other pallets state
-            T::CheckCertAllowed::check_cert_allowed(issuer, receiver)?;
-
-            // Verify rule MinReceivedCertToBeAbleToIssueCert
-            let issuer_idty_cert_meta = <StorageIdtyCertMeta<T, I>>::get(issuer);
-            ensure!(
-                issuer_idty_cert_meta.received_count
-                    >= T::MinReceivedCertToBeAbleToIssueCert::get(),
-                Error::<T, I>::NotEnoughCertReceived
-            );
-
-            // Verify rule MaxByIssuer
-            ensure!(
-                issuer_idty_cert_meta.issued_count < T::MaxByIssuer::get(),
-                Error::<T, I>::IssuedTooManyCert
-            );
-
-            // Verify rule CertPeriod
             let block_number = frame_system::pallet::Pallet::<T>::block_number();
-            ensure!(
-                block_number >= issuer_idty_cert_meta.next_issuable_on,
-                Error::<T, I>::NotRespectCertPeriod
-            );
-
+            Self::check_cert_allowed(who, issuer, receiver, block_number)?;
             Self::do_add_cert(block_number, issuer, receiver)
         }
 
@@ -524,6 +494,54 @@ pub mod pallet {
             }
             total_weight
         }
+
+        /// check cert allowed
+        // first internal checks
+        // then external checks
+        fn check_cert_allowed(
+            caller_key: T::AccountId,
+            issuer: T::IdtyIndex,
+            receiver: T::IdtyIndex,
+            block_number: T::BlockNumber,
+        ) -> DispatchResult {
+            // --- first internal checks
+            // 1. Forbid self cert
+            ensure!(issuer != receiver, Error::<T, I>::CannotCertifySelf);
+
+            // 2. Verify caller ownership
+            let issuer_owner_key =
+                T::OwnerKeyOf::convert(issuer).ok_or(Error::<T, I>::IssuerNotFound)?;
+            ensure!(issuer_owner_key == caller_key, DispatchError::BadOrigin);
+
+            // 3. Verify rule MinReceivedCertToBeAbleToIssueCert
+            // (this number can differ from the one necessary to be member)
+            let issuer_idty_cert_meta = <StorageIdtyCertMeta<T, I>>::get(issuer);
+            ensure!(
+                issuer_idty_cert_meta.received_count
+                    >= T::MinReceivedCertToBeAbleToIssueCert::get(),
+                Error::<T, I>::NotEnoughCertReceived
+            );
+
+            // 4. Verify rule MaxByIssuer
+            ensure!(
+                issuer_idty_cert_meta.issued_count < T::MaxByIssuer::get(),
+                Error::<T, I>::IssuedTooManyCert
+            );
+
+            // 5. Verify rule CertPeriod
+            ensure!(
+                block_number >= issuer_idty_cert_meta.next_issuable_on,
+                Error::<T, I>::NotRespectCertPeriod
+            );
+
+            // --- then external checks
+            // - issuer is member
+            // - receiver is confirmed
+            // - receiver is not revoked
+            T::CheckCertAllowed::check_cert_allowed(issuer, receiver)?;
+
+            Ok(())
+        }
     }
 }
 

pallets/duniter-wot/src/lib.rs

@@ -190,26 +190,12 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id
 {
     // check the following:
     // - issuer has identity
-    // - issuer identity is validated
+    // - issuer identity is member
     // - receiver has identity
-    // - receiver identity is confirmed or validated
-    // - receiver has membership
-    //
-    // /!\ do not check the following:
-    // - receiver has membership
-    // - issuer has membership
-    // this has the following consequences:
-    // - issuer can issue cert even if he lost his membership
-    //   (not renewed or passed below cert threshold and above again without claiming membership)
-    // this is counterintuitive behavior but not a big problem
-    //
-    // TODO to fix this strange behavior, we will have to make the tests
-    // (CheckCertAllowed and CheckMembershipCallAllowed) run on the relevant instance
-    // i.e. Cert for Wot, SmithCert for SmithWot...
-    // → see issue #136
+    // - receiver identity is confirmed and not revoked
     fn check_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> Result<(), DispatchError> {
         // issuer checks
-        // ensure issuer has validated identity
+        // ensure issuer is member
         if let Some(issuer_data) = pallet_identity::Pallet::<T>::identity(issuer) {
             ensure!(
                 issuer_data.status == IdtyStatus::Member,
@@ -218,15 +204,9 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id
         } else {
             return Err(Error::<T, I>::IdtyNotFound.into());
         }
-        // issue #136 this has to be done on the correct instance of membership pallet
-        // // ensure issuer has membership
-        // if pallet_membership::Pallet::<T, I>::membership(issuer).is_none() {
-        //     // improvement: give reason why issuer can not emit cert (not member)
-        //     return Err(Error::<T, I>::IssuerNotMember.into());
-        // }
 
         // receiver checks
-        // ensure receiver has confirmed or validated identity
+        // ensure receiver identity is confirmed and not revoked
         if let Some(receiver_data) = pallet_identity::Pallet::<T>::identity(receiver) {
             match receiver_data.status {
                 IdtyStatus::Unvalidated | IdtyStatus::Member | IdtyStatus::NotMember => {} // able to receive cert
@@ -236,13 +216,6 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id
         } else {
             return Err(Error::<T, I>::IdtyNotFound.into());
         }
-        // issue #136 this has to be done on the correct instance of membership pallet
-        // // ensure receiver has a membership or a pending membership
-        // if pallet_membership::Pallet::<T, I>::pending_membership(issuer).is_none()
-        //     && pallet_membership::Pallet::<T, I>::membership(issuer).is_none()
-        // {
-        //     return Err(Error::<T, I>::CertToUndefined.into());
-        // }
         Ok(())
     }
 }