From 2a46c45bd3efdd3542fdf9cd640fb036d60992ae Mon Sep 17 00:00:00 2001
From: Hugo Trentesaux <hugo@trentesaux.fr>
Date: Fri, 23 Sep 2022 09:46:31 +0200
Subject: [PATCH] wip merge membership traits

---
 pallets/duniter-wot/src/lib.rs       | 48 +++++++++++++++++-----------
 pallets/duniter-wot/src/mock.rs      |  8 ++---
 pallets/membership/src/lib.rs        | 29 +++--------------
 pallets/membership/src/mock.rs       |  4 +--
 primitives/membership/src/traits.rs  | 36 +++++++--------------
 runtime/common/src/pallets_config.rs |  8 ++---
 6 files changed, 51 insertions(+), 82 deletions(-)

diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index e84484e7b..5afc0ce52 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -97,6 +97,12 @@ pub mod pallet {
 
     #[pallet::error]
     pub enum Error<T, I = ()> {
+        /// Identity not allowed to claim membership
+        IdtyNotAllowedToClaimMembership,
+        /// Identity not allowed to request membership
+        IdtyNotAllowedToRequestMembership,
+        /// Identity not allowed to renew membership
+        IdtyNotAllowedToRenewMembership,
         /// Identity creation period not respected
         IdtyCreationPeriodNotRespected,
         /// Not enough received certifications to create identity
@@ -215,36 +221,40 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id
     }
 }
 
-impl<T: Config<I>, I: 'static> sp_membership::traits::IsIdtyAllowedToClaimMembership<IdtyIndex>
+impl<T: Config<I>, I: 'static> sp_membership::traits::CheckIdtyAllowedMembership<IdtyIndex>
     for Pallet<T, I>
 {
-    fn is_idty_allowed_to_claim_membership(idty_index: &IdtyIndex) -> bool {
+    fn check_idty_allowed_to_claim_membership(idty_index: &IdtyIndex) -> Result<(), DispatchError> {
         let idty_cert_meta = pallet_certification::Pallet::<T, I>::idty_cert_meta(idty_index);
-        idty_cert_meta.received_count >= T::MinCertForMembership::get() as u32
+        ensure!(
+            idty_cert_meta.received_count >= T::MinCertForMembership::get() as u32,
+            Error::<T, I>::IdtyNotAllowedToClaimMembership
+        );
+        Ok(())
     }
-}
 
-impl<T: Config<I>, I: 'static> sp_membership::traits::IsIdtyAllowedToRenewMembership<IdtyIndex>
-    for Pallet<T, I>
-{
-    fn is_idty_allowed_to_renew_membership(idty_index: &IdtyIndex) -> bool {
+    fn check_idty_allowed_to_renew_membership(idty_index: &IdtyIndex) -> Result<(), DispatchError> {
         if let Some(idty_value) = pallet_identity::Pallet::<T>::identity(idty_index) {
-            idty_value.status == IdtyStatus::Validated
-        } else {
-            false
+            ensure!(
+                idty_value.status == IdtyStatus::Validated,
+                Error::<T, I>::IdtyNotAllowedToRenewMembership
+            );
         }
+        // FIXME what if identity not found?
+        Ok(())
     }
-}
 
-impl<T: Config<I>, I: 'static> sp_membership::traits::IsIdtyAllowedToRequestMembership<IdtyIndex>
-    for Pallet<T, I>
-{
-    fn is_idty_allowed_to_request_membership(idty_index: &IdtyIndex) -> bool {
+    fn check_idty_allowed_to_request_membership(
+        idty_index: &IdtyIndex,
+    ) -> Result<(), DispatchError> {
         if let Some(idty_value) = pallet_identity::Pallet::<T>::identity(idty_index) {
-            T::IsSubWot::get() && idty_value.status == IdtyStatus::Validated
-        } else {
-            false
+            ensure!(
+                T::IsSubWot::get() && idty_value.status == IdtyStatus::Validated,
+                Error::<T, I>::IdtyNotAllowedToRequestMembership
+            );
         }
+        // FIXME what if identity not found?
+        Ok(())
     }
 }
 
diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs
index d26889790..5552ca3bc 100644
--- a/pallets/duniter-wot/src/mock.rs
+++ b/pallets/duniter-wot/src/mock.rs
@@ -142,9 +142,7 @@ parameter_types! {
 }
 
 impl pallet_membership::Config<Instance1> for Test {
-    type IsIdtyAllowedToClaimMembership = DuniterWot;
-    type IsIdtyAllowedToRenewMembership = DuniterWot;
-    type IsIdtyAllowedToRequestMembership = DuniterWot;
+    type CheckIdtyAllowedMembership = DuniterWot;
     type Event = Event;
     type IdtyId = IdtyIndex;
     type IdtyIdOf = IdentityIndexOf<Self>;
@@ -196,9 +194,7 @@ parameter_types! {
 }
 
 impl pallet_membership::Config<Instance2> for Test {
-    type IsIdtyAllowedToClaimMembership = SmithsSubWot;
-    type IsIdtyAllowedToRenewMembership = SmithsSubWot;
-    type IsIdtyAllowedToRequestMembership = SmithsSubWot;
+    type CheckIdtyAllowedMembership = SmithsSubWot;
     type Event = Event;
     type IdtyId = IdtyIndex;
     type IdtyIdOf = IdentityIndexOf<Self>;
diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs
index 4c9d84b24..e3634f5d4 100644
--- a/pallets/membership/src/lib.rs
+++ b/pallets/membership/src/lib.rs
@@ -59,12 +59,8 @@ pub mod pallet {
 
     #[pallet::config]
     pub trait Config<I: 'static = ()>: frame_system::Config {
-        /// Ask the runtime if the identity can claim the membership
-        type IsIdtyAllowedToClaimMembership: IsIdtyAllowedToClaimMembership<Self::IdtyId>;
-        /// Ask the runtime if the identity can renew his membership
-        type IsIdtyAllowedToRenewMembership: IsIdtyAllowedToRenewMembership<Self::IdtyId>;
-        /// Ask the runtime if the identity can request the membership
-        type IsIdtyAllowedToRequestMembership: IsIdtyAllowedToRequestMembership<Self::IdtyId>;
+        /// Ask the runtime if the identity can perform membership operations
+        type CheckIdtyAllowedMembership: CheckIdtyAllowedMembership<Self::IdtyId>;
         /// Because this pallet emits events, it depends on the runtime's definition of an event.
         type Event: From<Event<Self, I>> + IsType<<Self as frame_system::Config>::Event>;
         /// Something that identifies an identity
@@ -160,12 +156,6 @@ pub mod pallet {
 
     #[pallet::error]
     pub enum Error<T, I = ()> {
-        /// Identity not allowed to claim membership
-        IdtyNotAllowedToClaimMembership,
-        /// Identity not allowed to request membership
-        IdtyNotAllowedToRequestMembership,
-        /// Identity not allowed to renew membership
-        IdtyNotAllowedToRenewMembership,
         /// Invalid meta data
         InvalidMetaData,
         /// Identity id not found
@@ -221,10 +211,7 @@ pub mod pallet {
             if !metadata.validate(&who) {
                 return Err(Error::<T, I>::InvalidMetaData.into());
             }
-            if !T::IsIdtyAllowedToRequestMembership::is_idty_allowed_to_request_membership(&idty_id)
-            {
-                return Err(Error::<T, I>::IdtyNotAllowedToRequestMembership.into());
-            }
+            T::CheckIdtyAllowedMembership::check_idty_allowed_to_request_membership(&idty_id)?;
 
             Self::do_request_membership(idty_id, metadata)
         }
@@ -242,10 +229,7 @@ pub mod pallet {
                 Error::<T, I>::MembershipAlreadyAcquired
             );
 
-            ensure!(
-                T::IsIdtyAllowedToClaimMembership::is_idty_allowed_to_claim_membership(&idty_id),
-                Error::<T, I>::IdtyNotAllowedToClaimMembership
-            );
+            T::CheckIdtyAllowedMembership::check_idty_allowed_to_claim_membership(&idty_id)?;
 
             let metadata = PendingMembership::<T, I>::take(&idty_id)
                 .ok_or(Error::<T, I>::MembershipRequestNotFound)?;
@@ -271,10 +255,7 @@ pub mod pallet {
                 Error::<T, I>::MembershipNotFound
             );
 
-            ensure!(
-                T::IsIdtyAllowedToRenewMembership::is_idty_allowed_to_renew_membership(&idty_id),
-                Error::<T, I>::IdtyNotAllowedToRenewMembership,
-            );
+            T::CheckIdtyAllowedMembership::check_idty_allowed_to_renew_membership(&idty_id)?;
 
             let _ = Self::do_renew_membership(idty_id);
 
diff --git a/pallets/membership/src/mock.rs b/pallets/membership/src/mock.rs
index 7002e13db..5710aa341 100644
--- a/pallets/membership/src/mock.rs
+++ b/pallets/membership/src/mock.rs
@@ -83,9 +83,7 @@ parameter_types! {
 }
 
 impl pallet_membership::Config for Test {
-    type IsIdtyAllowedToClaimMembership = ();
-    type IsIdtyAllowedToRenewMembership = ();
-    type IsIdtyAllowedToRequestMembership = ();
+    type CheckIdtyAllowedMembership = ();
     type Event = Event;
     type IdtyId = IdtyId;
     type IdtyIdOf = ConvertInto;
diff --git a/primitives/membership/src/traits.rs b/primitives/membership/src/traits.rs
index eaaeb98fb..5a6e7df1d 100644
--- a/primitives/membership/src/traits.rs
+++ b/primitives/membership/src/traits.rs
@@ -14,35 +14,23 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with Duniter-v2S. If not, see <https://www.gnu.org/licenses/>.
 
-use frame_support::pallet_prelude::Weight;
+use frame_support::pallet_prelude::*;
 
-pub trait IsIdtyAllowedToClaimMembership<IdtyId> {
-    fn is_idty_allowed_to_claim_membership(idty_id: &IdtyId) -> bool;
+pub trait CheckIdtyAllowedMembership<IdtyId> {
+    fn check_idty_allowed_to_claim_membership(idty_id: &IdtyId) -> Result<(), DispatchError>;
+    fn check_idty_allowed_to_renew_membership(idty_id: &IdtyId) -> Result<(), DispatchError>;
+    fn check_idty_allowed_to_request_membership(idty_id: &IdtyId) -> Result<(), DispatchError>;
 }
 
-impl<IdtyId> IsIdtyAllowedToClaimMembership<IdtyId> for () {
-    fn is_idty_allowed_to_claim_membership(_: &IdtyId) -> bool {
-        true
+impl<IdtyId> CheckIdtyAllowedMembership<IdtyId> for () {
+    fn check_idty_allowed_to_claim_membership(_: &IdtyId) -> Result<(), DispatchError> {
+        Ok(())
     }
-}
-
-pub trait IsIdtyAllowedToRenewMembership<IdtyId> {
-    fn is_idty_allowed_to_renew_membership(idty_id: &IdtyId) -> bool;
-}
-
-impl<IdtyId> IsIdtyAllowedToRenewMembership<IdtyId> for () {
-    fn is_idty_allowed_to_renew_membership(_: &IdtyId) -> bool {
-        true
+    fn check_idty_allowed_to_renew_membership(_: &IdtyId) -> Result<(), DispatchError> {
+        Ok(())
     }
-}
-
-pub trait IsIdtyAllowedToRequestMembership<IdtyId> {
-    fn is_idty_allowed_to_request_membership(idty_id: &IdtyId) -> bool;
-}
-
-impl<IdtyId> IsIdtyAllowedToRequestMembership<IdtyId> for () {
-    fn is_idty_allowed_to_request_membership(_: &IdtyId) -> bool {
-        true
+    fn check_idty_allowed_to_request_membership(_: &IdtyId) -> Result<(), DispatchError> {
+        Ok(())
     }
 }
 
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index c5c6418fe..895dfdc8a 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/runtime/common/src/pallets_config.rs
@@ -444,9 +444,7 @@ macro_rules! pallets_config {
         }
 
         impl pallet_membership::Config<frame_support::instances::Instance1> for Runtime {
-            type IsIdtyAllowedToClaimMembership = Wot;
-            type IsIdtyAllowedToRenewMembership = Wot;
-            type IsIdtyAllowedToRequestMembership = Wot;
+            type CheckIdtyAllowedMembership = Wot;
             type Event = Event;
             type IdtyId = IdtyIndex;
             type IdtyIdOf = common_runtime::providers::IdentityIndexOf<Self>;
@@ -480,9 +478,7 @@ macro_rules! pallets_config {
         }
 
         impl pallet_membership::Config<Instance2> for Runtime {
-            type IsIdtyAllowedToClaimMembership = SmithsSubWot;
-            type IsIdtyAllowedToRenewMembership = SmithsSubWot;
-            type IsIdtyAllowedToRequestMembership = SmithsSubWot;
+            type CheckIdtyAllowedMembership = SmithsSubWot;
             type Event = Event;
             type IdtyId = IdtyIndex;
             type IdtyIdOf = common_runtime::providers::IdentityIndexOf<Self>;
-- 
GitLab