diff --git a/pallets/certification/src/lib.rs b/pallets/certification/src/lib.rs index 6074908e27aaf08423bf0f46dba96892f73c8a76..fd60d814e7e8cef3591f1e174c434637267ec90e 100644 --- a/pallets/certification/src/lib.rs +++ b/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(()) + } } } diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs index 3ffbcf506f7e623cb162a8c9bb8487021a858b82..4ecdc454c49719a2471800c628ead97f905aaff7 100644 --- a/pallets/duniter-wot/src/lib.rs +++ b/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(()) } }