From 7f72977adab31d8ef86ddbc837934c6fb90834d2 Mon Sep 17 00:00:00 2001 From: Hugo Trentesaux <hugo.trentesaux@lilo.org> Date: Sat, 12 Nov 2022 22:34:50 +0100 Subject: [PATCH] Replace bool by Result<(), dispatchError> for checks (nodes/rust/duniter-v2s!112) * fix same typo * fix typo * revert variable name for more explicit * use exhaustive explicit match for idtystatus * review tuxmain * review * fix fixme * wip fix tests * wip merge membership traits * wip rename check traits * wip replace cert allowed check * wip replace identity removal check * wip replace adress change check * wip replace identity validation check * wip replace identity confirm check * wip fix generic type * wip try replace identity creation check --- pallets/certification/src/lib.rs | 9 +- pallets/certification/src/mock.rs | 2 +- pallets/certification/src/traits.rs | 12 +- pallets/duniter-wot/src/lib.rs | 159 +++++++++++++++++---------- pallets/duniter-wot/src/mock.rs | 14 +-- pallets/duniter-wot/src/tests.rs | 12 +- pallets/identity/src/lib.rs | 35 ++---- pallets/identity/src/mock.rs | 2 +- pallets/identity/src/traits.rs | 45 ++++---- pallets/membership/src/lib.rs | 29 +---- pallets/membership/src/mock.rs | 4 +- primitives/membership/src/traits.rs | 36 ++---- runtime/common/src/pallets_config.rs | 14 +-- 13 files changed, 176 insertions(+), 197 deletions(-) diff --git a/pallets/certification/src/lib.rs b/pallets/certification/src/lib.rs index 84007fc66..75d79258c 100644 --- a/pallets/certification/src/lib.rs +++ b/pallets/certification/src/lib.rs @@ -71,7 +71,7 @@ pub mod pallet { /// Something that give the owner key of an identity type OwnerKeyOf: Convert<Self::IdtyIndex, Option<Self::AccountId>>; /// - type IsCertAllowed: IsCertAllowed<Self::IdtyIndex>; + type CheckCertAllowed: CheckCertAllowed<Self::IdtyIndex>; #[pallet::constant] /// Maximum number of active certifications by issuer type MaxByIssuer: Get<u32>; @@ -243,8 +243,6 @@ pub mod pallet { pub enum Error<T, I = ()> { /// An identity cannot certify itself CannotCertifySelf, - /// Certification non autorisée - CertNotAllowed, /// This identity has already issued the maximum number of certifications IssuedTooManyCert, /// Issuer not found @@ -326,10 +324,7 @@ pub mod pallet { ensure!(issuer_owner_key == who, DispatchError::BadOrigin); // Verify compatibility with other pallets state - ensure!( - T::IsCertAllowed::is_cert_allowed(issuer, receiver), - Error::<T, I>::CertNotAllowed - ); + T::CheckCertAllowed::check_cert_allowed(issuer, receiver)?; // Verify rule MinReceivedCertToBeAbleToIssueCert let issuer_idty_cert_meta = <StorageIdtyCertMeta<T, I>>::get(issuer); diff --git a/pallets/certification/src/mock.rs b/pallets/certification/src/mock.rs index 066d3ebb0..68de7a87e 100644 --- a/pallets/certification/src/mock.rs +++ b/pallets/certification/src/mock.rs @@ -103,7 +103,7 @@ impl pallet_certification::Config for Test { type Event = Event; type IdtyIndex = IdtyIndex; type OwnerKeyOf = sp_runtime::traits::ConvertInto; - type IsCertAllowed = (); + type CheckCertAllowed = (); type MaxByIssuer = MaxByIssuer; type MinReceivedCertToBeAbleToIssueCert = MinReceivedCertToBeAbleToIssueCert; type OnNewcert = (); diff --git a/pallets/certification/src/traits.rs b/pallets/certification/src/traits.rs index 6c41d8c90..9a81bb3d0 100644 --- a/pallets/certification/src/traits.rs +++ b/pallets/certification/src/traits.rs @@ -14,13 +14,15 @@ // 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/>. -pub trait IsCertAllowed<IdtyIndex> { - fn is_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> bool; +use frame_support::pallet_prelude::*; + +pub trait CheckCertAllowed<IdtyIndex> { + fn check_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> Result<(), DispatchError>; } -impl<IdtyIndex> IsCertAllowed<IdtyIndex> for () { - fn is_cert_allowed(_issuer: IdtyIndex, _receiver: IdtyIndex) -> bool { - true +impl<IdtyIndex> CheckCertAllowed<IdtyIndex> for () { + fn check_cert_allowed(_issuer: IdtyIndex, _receiver: IdtyIndex) -> Result<(), DispatchError> { + Ok(()) } } diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs index cbcf635ee..1d195b096 100644 --- a/pallets/duniter-wot/src/lib.rs +++ b/pallets/duniter-wot/src/lib.rs @@ -92,36 +92,72 @@ pub mod pallet { true } } + + // ERRORS // + + #[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 + NotEnoughReceivedCertsToCreateIdty, + /// Max number of emitted certs reached + MaxEmittedCertsReached, + /// Not allowed to change identity address + NotAllowedToChangeIdtyAddress, + /// Not allowed to remove identity + NotAllowedToRemoveIdty, + /// Issuer can not emit cert because it is not validated + IssuerCanNotEmitCert, + /// Can not issue cert to unconfirmed identity + CertToUnconfirmedIdty, + /// Issuer or receiver not found + IdtyNotFound, + } } -impl<AccountId, T: Config<I>, I: 'static> pallet_identity::traits::EnsureIdtyCallAllowed<T> +impl<AccountId, T: Config<I>, I: 'static> pallet_identity::traits::CheckIdtyCallAllowed<T> for Pallet<T, I> where T: frame_system::Config<AccountId = AccountId> + pallet_membership::Config<I>, { - fn can_create_identity(creator: IdtyIndex) -> bool { + fn check_create_identity(creator: IdtyIndex) -> Result<(), DispatchError> { if !T::IsSubWot::get() { let cert_meta = pallet_certification::Pallet::<T, I>::idty_cert_meta(creator); - cert_meta.received_count >= T::MinCertForCreateIdtyRight::get() - && cert_meta.next_issuable_on <= frame_system::pallet::Pallet::<T>::block_number() - && cert_meta.issued_count < T::MaxByIssuer::get() - } else { - true + // perform all checks + ensure!( + cert_meta.received_count >= T::MinCertForCreateIdtyRight::get(), + Error::<T, I>::NotEnoughReceivedCertsToCreateIdty + ); + ensure!( + cert_meta.issued_count < T::MaxByIssuer::get(), + Error::<T, I>::MaxEmittedCertsReached + ); + ensure!( + cert_meta.next_issuable_on <= frame_system::pallet::Pallet::<T>::block_number(), + Error::<T, I>::IdtyCreationPeriodNotRespected + ); } + Ok(()) } - fn can_confirm_identity(idty_index: IdtyIndex) -> bool { + fn check_confirm_identity(idty_index: IdtyIndex) -> Result<(), DispatchError> { if !T::IsSubWot::get() { pallet_membership::Pallet::<T, I>::force_request_membership( RawOrigin::Root.into(), idty_index, Default::default(), ) - .is_ok() - } else { - true + .map_err(|e| e.error)?; } + Ok(()) } - fn can_validate_identity(idty_index: IdtyIndex) -> bool { + fn check_validate_identity(idty_index: IdtyIndex) -> Result<(), DispatchError> { if !T::IsSubWot::get() { // TODO replace this code by the commented one for distance feature /*let idty_cert_meta = pallet_certification::Pallet::<T, I>::idty_cert_meta(idty_index); @@ -130,81 +166,88 @@ where RawOrigin::Root.into(), Some(idty_index), ) - .is_ok() - } else { - true + .map_err(|e| e.error)?; } + Ok(()) } - fn can_change_identity_address(idty_index: IdtyIndex) -> bool { + fn check_change_identity_address(idty_index: IdtyIndex) -> Result<(), DispatchError> { if T::IsSubWot::get() { - !pallet_membership::Pallet::<T, I>::is_member(&idty_index) - } else { - true + ensure!( + !pallet_membership::Pallet::<T, I>::is_member(&idty_index), + Error::<T, I>::NotAllowedToChangeIdtyAddress + ); } + Ok(()) } - fn can_remove_identity(idty_index: IdtyIndex) -> bool { + fn check_remove_identity(idty_index: IdtyIndex) -> Result<(), DispatchError> { if T::IsSubWot::get() { - !pallet_membership::Pallet::<T, I>::is_member(&idty_index) - } else { - true + ensure!( + !pallet_membership::Pallet::<T, I>::is_member(&idty_index), + Error::<T, I>::NotAllowedToRemoveIdty + ); } + Ok(()) } } -impl<T: Config<I>, I: 'static> pallet_certification::traits::IsCertAllowed<IdtyIndex> +impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<IdtyIndex> for Pallet<T, I> { - fn is_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> bool { + fn check_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> Result<(), DispatchError> { if let Some(issuer_data) = pallet_identity::Pallet::<T>::identity(issuer) { - if issuer_data.status != IdtyStatus::Validated { - return false; - } - if let Some(receiver_data) = pallet_identity::Pallet::<T>::identity(receiver) { - match receiver_data.status { - IdtyStatus::ConfirmedByOwner | IdtyStatus::Validated => true, - IdtyStatus::Created => false, - } - } else { - // Receiver not found - false - } + ensure!( + issuer_data.status == IdtyStatus::Validated, + Error::<T, I>::IssuerCanNotEmitCert + ); } else { - // Issuer not found - false + return Err(Error::<T, I>::IdtyNotFound.into()); } + if let Some(receiver_data) = pallet_identity::Pallet::<T>::identity(receiver) { + match receiver_data.status { + IdtyStatus::ConfirmedByOwner | IdtyStatus::Validated => {} // able to receive cert + IdtyStatus::Created => return Err(Error::<T, I>::CertToUnconfirmedIdty.into()), + }; + } else { + return Err(Error::<T, I>::IdtyNotFound.into()); + } + Ok(()) } } -impl<T: Config<I>, I: 'static> sp_membership::traits::IsIdtyAllowedToClaimMembership<IdtyIndex> - for Pallet<T, I> -{ - fn is_idty_allowed_to_claim_membership(idty_index: &IdtyIndex) -> bool { +impl<T: Config<I>, I: 'static> sp_membership::traits::CheckCallAllowed<IdtyIndex> for Pallet<T, I> { + 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 + ensure!( + idty_value.status == IdtyStatus::Validated, + Error::<T, I>::IdtyNotAllowedToRenewMembership + ); } else { - false + return Err(Error::<T, I>::IdtyNotFound.into()); } + 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 + ensure!( + T::IsSubWot::get() && idty_value.status == IdtyStatus::Validated, + Error::<T, I>::IdtyNotAllowedToRequestMembership + ); } else { - false + return Err(Error::<T, I>::IdtyNotFound.into()); } + Ok(()) } } diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index d6872196f..f90f159b2 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -122,7 +122,7 @@ impl pallet_identity::Config for Test { type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod; type ConfirmPeriod = ConfirmPeriod; type Event = Event; - type EnsureIdtyCallAllowed = (DuniterWot, SmithsSubWot); + type CheckIdtyCallAllowed = (DuniterWot, SmithsSubWot); type IdtyCreationPeriod = IdtyCreationPeriod; type IdtyData = (); type IdtyNameValidator = IdtyNameValidatorTestImpl; @@ -142,9 +142,7 @@ parameter_types! { } impl pallet_membership::Config<Instance1> for Test { - type IsIdtyAllowedToClaimMembership = DuniterWot; - type IsIdtyAllowedToRenewMembership = DuniterWot; - type IsIdtyAllowedToRequestMembership = DuniterWot; + type CheckCallAllowed = DuniterWot; type Event = Event; type IdtyId = IdtyIndex; type IdtyIdOf = IdentityIndexOf<Self>; @@ -167,7 +165,7 @@ impl pallet_certification::Config<Instance1> for Test { type Event = Event; type IdtyIndex = IdtyIndex; type OwnerKeyOf = Identity; - type IsCertAllowed = DuniterWot; + type CheckCertAllowed = DuniterWot; type MaxByIssuer = MaxByIssuer; type MinReceivedCertToBeAbleToIssueCert = MinReceivedCertToBeAbleToIssueCert; type OnNewcert = DuniterWot; @@ -196,9 +194,7 @@ parameter_types! { } impl pallet_membership::Config<Instance2> for Test { - type IsIdtyAllowedToClaimMembership = SmithsSubWot; - type IsIdtyAllowedToRenewMembership = SmithsSubWot; - type IsIdtyAllowedToRequestMembership = SmithsSubWot; + type CheckCallAllowed = SmithsSubWot; type Event = Event; type IdtyId = IdtyIndex; type IdtyIdOf = IdentityIndexOf<Self>; @@ -221,7 +217,7 @@ impl pallet_certification::Config<Instance2> for Test { type Event = Event; type IdtyIndex = IdtyIndex; type OwnerKeyOf = Identity; - type IsCertAllowed = SmithsSubWot; + type CheckCertAllowed = SmithsSubWot; type MaxByIssuer = SmithsMaxByIssuer; type MinReceivedCertToBeAbleToIssueCert = SmithsMinReceivedCertToBeAbleToIssueCert; type OnNewcert = SmithsSubWot; diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs index e40134c57..91c171561 100644 --- a/pallets/duniter-wot/src/tests.rs +++ b/pallets/duniter-wot/src/tests.rs @@ -16,8 +16,9 @@ use crate::mock::*; use crate::mock::{Identity, System}; +use crate::pallet as pallet_duniter_wot; use codec::Encode; -use frame_support::instances::Instance1; +use frame_support::instances::{Instance1, Instance2}; use frame_support::{assert_noop, assert_ok}; use pallet_identity::{ IdtyName, IdtyStatus, NewOwnerKeyPayload, RevocationPayload, NEW_OWNER_KEY_PAYLOAD_PREFIX, @@ -46,9 +47,10 @@ fn test_creator_not_allowed_to_create_idty() { // Alice should not be able to create an identity before block #2 // because Alice.next_issuable_on = 2 + // but the true reason is that alice did not receive enough certs assert_noop!( Identity::create_identity(Origin::signed(1), 4), - pallet_identity::Error::<Test>::CreatorNotAllowedToCreateIdty + pallet_duniter_wot::Error::<Test, Instance1>::NotEnoughReceivedCertsToCreateIdty ); }); } @@ -114,7 +116,7 @@ fn test_smith_member_cant_change_its_idty_address() { 13, TestSignature(13, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) ), - pallet_identity::Error::<Test>::NotAllowedToChangeIdtyAddress + pallet_duniter_wot::Error::<Test, Instance2>::NotAllowedToChangeIdtyAddress ); }); } @@ -137,7 +139,7 @@ fn test_smith_member_cant_revoke_its_idty() { 3, TestSignature(3, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) ), - pallet_identity::Error::<Test>::NotAllowedToRemoveIdty + pallet_duniter_wot::Error::<Test, Instance2>::NotAllowedToRemoveIdty ); }); } @@ -269,7 +271,7 @@ fn test_idty_membership_expire_them_requested() { // Alice can't renew her cert to Charlie assert_noop!( Cert::add_cert(Origin::signed(1), 1, 3), - pallet_certification::Error::<Test, Instance1>::CertNotAllowed + pallet_duniter_wot::Error::<Test, Instance1>::IdtyNotFound ); }); } diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index 07c81a031..961b8e2f8 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -72,7 +72,7 @@ pub mod pallet { type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>; /// Management of the authorizations of the different calls. /// The default implementation allows everything. - type EnsureIdtyCallAllowed: EnsureIdtyCallAllowed<Self>; + type CheckIdtyCallAllowed: CheckIdtyCallAllowed<Self>; #[pallet::constant] /// Minimum duration between the creation of 2 identities by the same creator type IdtyCreationPeriod: Get<Self::BlockNumber>; @@ -271,9 +271,8 @@ pub mod pallet { return Err(Error::<T>::IdtyAlreadyCreated.into()); } - if !T::EnsureIdtyCallAllowed::can_create_identity(creator) { - return Err(Error::<T>::CreatorNotAllowedToCreateIdty.into()); - } + // run checks for identity creation + T::CheckIdtyCallAllowed::check_create_identity(creator)?; let block_number = frame_system::pallet::Pallet::<T>::block_number(); @@ -341,9 +340,7 @@ pub mod pallet { if <IdentitiesNames<T>>::contains_key(&idty_name) { return Err(Error::<T>::IdtyNameAlreadyExist.into()); } - if !T::EnsureIdtyCallAllowed::can_confirm_identity(idty_index) { - return Err(Error::<T>::NotAllowedToConfirmIdty.into()); - } + T::CheckIdtyCallAllowed::check_confirm_identity(idty_index)?; // Apply phase // idty_value.status = IdtyStatus::ConfirmedByOwner; @@ -372,9 +369,7 @@ pub mod pallet { match idty_value.status { IdtyStatus::Created => return Err(Error::<T>::IdtyNotConfirmedByOwner.into()), IdtyStatus::ConfirmedByOwner => { - if !T::EnsureIdtyCallAllowed::can_validate_identity(idty_index) { - return Err(Error::<T>::NotAllowedToValidateIdty.into()); - } + T::CheckIdtyCallAllowed::check_validate_identity(idty_index)?; } IdtyStatus::Validated => return Err(Error::<T>::IdtyAlreadyValidated.into()), } @@ -416,10 +411,7 @@ pub mod pallet { Error::<T>::OwnerKeyAlreadyUsed ); - ensure!( - T::EnsureIdtyCallAllowed::can_change_identity_address(idty_index), - Error::<T>::NotAllowedToChangeIdtyAddress - ); + T::CheckIdtyCallAllowed::check_change_identity_address(idty_index)?; let block_number = frame_system::Pallet::<T>::block_number(); let maybe_old_old_owner_key = @@ -506,10 +498,7 @@ pub mod pallet { Error::<T>::InvalidRevocationKey ); - ensure!( - T::EnsureIdtyCallAllowed::can_remove_identity(idty_index), - Error::<T>::NotAllowedToRemoveIdty - ); + T::CheckIdtyCallAllowed::check_remove_identity(idty_index)?; let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); let revocation_payload = RevocationPayload { @@ -579,8 +568,6 @@ pub mod pallet { #[pallet::error] pub enum Error<T> { - /// Creator not allowed to create identities - CreatorNotAllowedToCreateIdty, /// Identity already confirmed IdtyAlreadyConfirmed, /// Identity already created @@ -611,14 +598,6 @@ pub mod pallet { InvalidRevocationKey, /// Revocation payload signature is invalid InvalidRevocationSig, - /// Identity not allowed to change address - NotAllowedToChangeIdtyAddress, - /// Not allowed to confirm identity - NotAllowedToConfirmIdty, - /// Not allowed to remove identity - NotAllowedToRemoveIdty, - /// Not allowed to validate identity - NotAllowedToValidateIdty, /// Identity creation period is not respected NotRespectIdtyCreationPeriod, /// Not the same identity name diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index 535e06726..6825ebf66 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -94,7 +94,7 @@ impl pallet_identity::Config for Test { type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod; type ConfirmPeriod = ConfirmPeriod; type Event = Event; - type EnsureIdtyCallAllowed = (); + type CheckIdtyCallAllowed = (); type IdtyCreationPeriod = IdtyCreationPeriod; type IdtyData = (); type IdtyNameValidator = IdtyNameValidatorTestImpl; diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs index da9ddf71c..cc6230e4c 100644 --- a/pallets/identity/src/traits.rs +++ b/pallets/identity/src/traits.rs @@ -19,36 +19,35 @@ use frame_support::pallet_prelude::*; use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::Saturating; -pub trait EnsureIdtyCallAllowed<T: Config> { - fn can_create_identity(creator: T::IdtyIndex) -> bool; - fn can_confirm_identity(idty_index: T::IdtyIndex) -> bool; - fn can_validate_identity(idty_index: T::IdtyIndex) -> bool; - fn can_change_identity_address(idty_index: T::IdtyIndex) -> bool; - fn can_remove_identity(idty_index: T::IdtyIndex) -> bool; +pub trait CheckIdtyCallAllowed<T: Config> { + fn check_create_identity(creator: T::IdtyIndex) -> Result<(), DispatchError>; + fn check_confirm_identity(idty_index: T::IdtyIndex) -> Result<(), DispatchError>; + fn check_validate_identity(idty_index: T::IdtyIndex) -> Result<(), DispatchError>; + fn check_change_identity_address(idty_index: T::IdtyIndex) -> Result<(), DispatchError>; + fn check_remove_identity(idty_index: T::IdtyIndex) -> Result<(), DispatchError>; } #[impl_for_tuples(5)] -#[allow(clippy::let_and_return)] -impl<T: Config> EnsureIdtyCallAllowed<T> for Tuple { - fn can_create_identity(creator: T::IdtyIndex) -> bool { - for_tuples!( #( if !Tuple::can_create_identity(creator) { return false; } )* ); - true +impl<T: Config> CheckIdtyCallAllowed<T> for Tuple { + fn check_create_identity(creator: T::IdtyIndex) -> Result<(), DispatchError> { + for_tuples!( #( Tuple::check_create_identity(creator)?; )* ); + Ok(()) } - fn can_confirm_identity(idty_index: T::IdtyIndex) -> bool { - for_tuples!( #( if !Tuple::can_confirm_identity(idty_index) { return false; } )* ); - true + fn check_confirm_identity(idty_index: T::IdtyIndex) -> Result<(), DispatchError> { + for_tuples!( #( Tuple::check_confirm_identity(idty_index)?; )* ); + Ok(()) } - fn can_validate_identity(idty_index: T::IdtyIndex) -> bool { - for_tuples!( #( if !Tuple::can_validate_identity(idty_index) { return false; } )* ); - true + fn check_validate_identity(idty_index: T::IdtyIndex) -> Result<(), DispatchError> { + for_tuples!( #( Tuple::check_validate_identity(idty_index)?; )* ); + Ok(()) } - fn can_change_identity_address(idty_index: T::IdtyIndex) -> bool { - for_tuples!( #( if !Tuple::can_change_identity_address(idty_index) { return false; } )* ); - true + fn check_change_identity_address(idty_index: T::IdtyIndex) -> Result<(), DispatchError> { + for_tuples!( #( Tuple::check_change_identity_address(idty_index)?; )* ); + Ok(()) } - fn can_remove_identity(idty_index: T::IdtyIndex) -> bool { - for_tuples!( #( if !Tuple::can_remove_identity(idty_index) { return false; } )* ); - true + fn check_remove_identity(idty_index: T::IdtyIndex) -> Result<(), DispatchError> { + for_tuples!( #( Tuple::check_remove_identity(idty_index)?; )* ); + Ok(()) } } diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs index 4c9d84b24..aebd602ce 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 whether the identity can perform membership operations + type CheckCallAllowed: CheckCallAllowed<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::CheckCallAllowed::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::CheckCallAllowed::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::CheckCallAllowed::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..b292910e0 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 CheckCallAllowed = (); 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..81421eb8a 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 CheckCallAllowed<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> CheckCallAllowed<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 cdcaa4944..974db7fa9 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -430,7 +430,7 @@ macro_rules! pallets_config { type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod; type ConfirmPeriod = ConfirmPeriod; type Event = Event; - type EnsureIdtyCallAllowed = (Wot, SmithsSubWot); + type CheckIdtyCallAllowed = (Wot, SmithsSubWot); type IdtyCreationPeriod = IdtyCreationPeriod; type IdtyData = IdtyData; type IdtyIndex = IdtyIndex; @@ -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 CheckCallAllowed = Wot; type Event = Event; type IdtyId = IdtyIndex; type IdtyIdOf = common_runtime::providers::IdentityIndexOf<Self>; @@ -461,7 +459,7 @@ macro_rules! pallets_config { type Event = Event; type IdtyIndex = IdtyIndex; type OwnerKeyOf = Identity; - type IsCertAllowed = Wot; + type CheckCertAllowed = Wot; type MaxByIssuer = MaxByIssuer; type MinReceivedCertToBeAbleToIssueCert = MinReceivedCertToBeAbleToIssueCert; type OnNewcert = Wot; @@ -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 CheckCallAllowed = SmithsSubWot; type Event = Event; type IdtyId = IdtyIndex; type IdtyIdOf = common_runtime::providers::IdentityIndexOf<Self>; @@ -497,7 +493,7 @@ macro_rules! pallets_config { type Event = Event; type IdtyIndex = IdtyIndex; type OwnerKeyOf = Identity; - type IsCertAllowed = SmithsSubWot; + type CheckCertAllowed = SmithsSubWot; type MaxByIssuer = SmithMaxByIssuer; type MinReceivedCertToBeAbleToIssueCert = SmithMinReceivedCertToBeAbleToIssueCert; type OnNewcert = SmithsSubWot; -- GitLab