cargo check ok

pallets/distance/src/lib.rs

@@ -37,6 +37,7 @@ use pallet_authority_members::SessionIndex;
 use sp_distance::{InherentError, INHERENT_IDENTIFIER};
 use sp_inherents::{InherentData, InherentIdentifier};
 use sp_std::convert::TryInto;
+use sp_std::prelude::*;
 
 type IdtyIndex = u32;
 
@@ -81,6 +82,8 @@ pub mod pallet {
         #[pallet::constant]
         type MinAccessibleReferees: Get<Perbill>;
         /// Number of session to keep a positive evaluation result
+        // (antispam mechanism)
+        #[pallet::constant]
         type ResultExpiration: Get<u32>;
         /// The overarching event type.
         type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
@@ -131,18 +134,34 @@ pub mod pallet {
     pub type EvaluationBlock<T: Config> =
         StorageValue<_, <T as frame_system::Config>::Hash, ValueQuery>;
 
-    /// Distance evaluation status by identity
+    /// Pending evaluation requests
     ///
-    /// * `.0` is the account who requested an evaluation and reserved the price,
+    /// account who requested an evaluation and reserved the price,
     ///   for whom the price will be unreserved or slashed when the evaluation completes.
-    /// * `.1` is the status of the evaluation.
     #[pallet::storage]
-    #[pallet::getter(fn identity_distance_status)]
-    pub type IdentityDistanceStatus<T: Config> = StorageMap<
+    #[pallet::getter(fn pending_evaluation_request)]
+    pub type PendingEvaluationRequest<T: Config> = StorageMap<
         _,
         Twox64Concat,
         <T as pallet_identity::Config>::IdtyIndex,
-        (<T as frame_system::Config>::AccountId, DistanceStatus),
+        <T as frame_system::Config>::AccountId,
+        OptionQuery,
+    >;
+
+    /// Valid evaluation results
+    #[pallet::storage]
+    #[pallet::getter(fn valid_evaluation_result)]
+    pub type ValidEvaluationResult<T: Config> =
+        StorageMap<_, Twox64Concat, <T as pallet_identity::Config>::IdtyIndex, (), OptionQuery>;
+
+    /// Valid evaluation expiry blocks
+    #[pallet::storage]
+    #[pallet::getter(fn valid_evaluation_expire_on)]
+    pub type ValidEvaluationExpireOn<T: Config> = StorageMap<
+        _,
+        Twox64Concat,
+        u32,
+        Vec<<T as pallet_identity::Config>::IdtyIndex>,
         OptionQuery,
     >;
 
@@ -165,9 +184,9 @@ pub mod pallet {
             idty_index: T::IdtyIndex,
             who: T::AccountId,
         },
-        /// A distance evaluation was updated.
+        /// A distance evaluation was updated. // TODO refac
         EvaluationUpdated { evaluator: T::AccountId },
-        /// A distance status was forced.
+        /// A distance status was forced. // TODO check if necessary to differenciate
         EvaluationStatusForced {
             idty_index: T::IdtyIndex,
             status: Option<(<T as frame_system::Config>::AccountId, DistanceStatus)>,
@@ -187,7 +206,13 @@ pub mod pallet {
         /// No author for this block.
         NoAuthor,
         /// Caller has no identity.
-        NoIdentity,
+        CallerHasNoIdentity,
+        /// Caller identity not found.
+        CallerIdentityNotFound,
+        /// Caller not member.
+        CallerNotMember,
+        /// Target identity not found.
+        TargetIdentityNotFound,
         /// Evaluation queue is full.
         QueueFull,
         /// Too many evaluators in the current evaluation pool.
@@ -195,7 +220,10 @@ pub mod pallet {
         /// Evaluation result has a wrong length.
         WrongResultLength,
         /// Targeted distance evaluation request is only possible for an unvalidated identity
-        DistanceRequestNotAllowed,
+        DistanceRequestOnlyAllowedForUnvalidated,
+        /// Can not request distance evaluation when a valid result has already been published recently
+        // (antispam)
+        ValidDistanceResultAlreadyAvailable,
     }
 
     #[pallet::hooks]
@@ -227,18 +255,9 @@ pub mod pallet {
         pub fn request_distance_evaluation(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
             let who = ensure_signed(origin)?;
 
-            let idty =
-                pallet_identity::IdentityIndexOf::<T>::get(&who).ok_or(Error::<T>::NoIdentity)?;
-
-            // TODO is it necessary to check that the same account performed the request?
-            // TODO what if the distance status is existing but valid?
-            ensure!(
-                IdentityDistanceStatus::<T>::get(idty)
-                    != Some((who.clone(), DistanceStatus::Pending)),
-                Error::<T>::AlreadyInEvaluation
-            );
+            let idty = Self::check_request_distance_evaluation_self(&who)?;
 
-            Pallet::<T>::do_request_distance_evaluation(who, idty)?;
+            Pallet::<T>::do_request_distance_evaluation(&who, idty)?;
             Ok(().into())
         }
 
@@ -252,27 +271,9 @@ pub mod pallet {
         ) -> DispatchResultWithPostInfo {
             let who = ensure_signed(origin)?;
 
-            // check that the caller has an identity (TODO is this necessary ?)
-            // let _ =
-            //     pallet_identity::IdentityIndexOf::<T>::get(&who).ok_or(Error::<T>::NoIdentity)?;
-
-            // get the identity value of the target
-            let target_idty =
-                pallet_identity::Identities::<T>::get(target).ok_or(Error::<T>::NoIdentity)?;
+            Self::check_request_distance_evaluation_for(&who, target)?;
 
-            // check that target is unvalidated
-            ensure!(
-                target_idty.status == pallet_identity::IdtyStatus::Unvalidated,
-                Error::<T>::DistanceRequestNotAllowed
-            );
-
-            // check that no distance status is already there
-            ensure!(
-                IdentityDistanceStatus::<T>::get(target).is_none(),
-                Error::<T>::AlreadyInEvaluation
-            );
-
-            Pallet::<T>::do_request_distance_evaluation(who, target)?;
+            Pallet::<T>::do_request_distance_evaluation(&who, target)?;
             Ok(().into())
         }
 
@@ -322,18 +323,13 @@ pub mod pallet {
         /// * `status.1` is the status of the evaluation.
         #[pallet::call_index(3)]
         #[pallet::weight(<T as pallet::Config>::WeightInfo::force_set_distance_status())]
-        pub fn force_set_distance_status(
+        pub fn force_valid_distance_status(
             origin: OriginFor<T>,
             identity: <T as pallet_identity::Config>::IdtyIndex,
-            status: Option<(<T as frame_system::Config>::AccountId, DistanceStatus)>,
         ) -> DispatchResult {
             ensure_root(origin)?;
 
-            Self::do_set_distance_status(identity, status.clone());
-            Self::deposit_event(Event::EvaluationStatusForced {
-                idty_index: identity,
-                status,
-            });
+            Self::do_valid_distance_status(identity);
             Ok(())
         }
     }
@@ -402,9 +398,74 @@ pub mod pallet {
             }
         }
 
+        /// check that request distance evaluation is allowed
+        fn check_request_distance_evaluation_self(
+            who: &T::AccountId,
+        ) -> Result<<T as pallet_identity::Config>::IdtyIndex, DispatchError> {
+            // caller has an identity
+            let idty_index = pallet_identity::IdentityIndexOf::<T>::get(who)
+                .ok_or(Error::<T>::CallerHasNoIdentity)?;
+            // TODO some of the following can be moved to a "check idty call allowed" managed by wot
+            let idty = pallet_identity::Identities::<T>::get(idty_index)
+                .ok_or(Error::<T>::CallerIdentityNotFound)?;
+            // caller is member
+            ensure!(
+                idty.status == pallet_identity::IdtyStatus::NotMember
+                    || idty.status == pallet_identity::IdtyStatus::Member,
+                Error::<T>::CallerNotMember
+            );
+            Self::check_request_distance_evaluation_common(idty_index)?;
+            Ok(idty_index)
+        }
+
+        /// check that targeted request distance evaluation is allowed
+        fn check_request_distance_evaluation_for(
+            who: &T::AccountId,
+            target: <T as pallet_identity::Config>::IdtyIndex,
+        ) -> Result<(), DispatchError> {
+            // caller has an identity
+            let caller_idty_index = pallet_identity::IdentityIndexOf::<T>::get(&who)
+                .ok_or(Error::<T>::CallerHasNoIdentity)?;
+            // TODO some of the following can be moved to a "check idty call allowed" managed by wot
+            let caller_idty = pallet_identity::Identities::<T>::get(caller_idty_index)
+                .ok_or(Error::<T>::CallerIdentityNotFound)?;
+            // caller is member
+            ensure!(
+                caller_idty.status == pallet_identity::IdtyStatus::Member,
+                Error::<T>::CallerNotMember
+            );
+            // target has an identity
+            let target_idty = pallet_identity::Identities::<T>::get(target)
+                .ok_or(Error::<T>::TargetIdentityNotFound)?;
+            // target is unvalidated
+            ensure!(
+                target_idty.status == pallet_identity::IdtyStatus::Unvalidated,
+                Error::<T>::DistanceRequestOnlyAllowedForUnvalidated
+            );
+            Self::check_request_distance_evaluation_common(target)?;
+            Ok(())
+        }
+
+        // common checks between check_request_distance_evaluation _self and _for
+        fn check_request_distance_evaluation_common(
+            target: <T as pallet_identity::Config>::IdtyIndex,
+        ) -> Result<(), DispatchError> {
+            // no pending evaluation request
+            ensure!(
+                PendingEvaluationRequest::<T>::get(target).is_none(),
+                Error::<T>::AlreadyInEvaluation
+            );
+            // no valid status (antispam)
+            ensure!(
+                ValidEvaluationResult::<T>::get(target).is_none(),
+                Error::<T>::ValidDistanceResultAlreadyAvailable
+            );
+            Ok(())
+        }
+
         /// request distance evaluation in current pool
         fn do_request_distance_evaluation(
-            who: T::AccountId,
+            who: &T::AccountId,
             idty_index: <T as pallet_identity::Config>::IdtyIndex,
         ) -> Result<(), DispatchError> {
             Pallet::<T>::mutate_current_pool(
@@ -415,19 +476,19 @@ pub mod pallet {
                         Error::<T>::QueueFull
                     );
 
-                    T::Currency::reserve(&who, <T as Config>::EvaluationPrice::get())?;
+                    T::Currency::reserve(who, <T as Config>::EvaluationPrice::get())?;
 
                     current_pool
                         .evaluations
                         .try_push((idty_index, median::MedianAcc::new()))
                         .map_err(|_| Error::<T>::QueueFull)?;
 
-                    IdentityDistanceStatus::<T>::insert(
-                        idty_index,
-                        (&who, DistanceStatus::Pending),
-                    );
+                    PendingEvaluationRequest::<T>::insert(idty_index, who);
 
-                    Self::deposit_event(Event::EvaluationRequested { idty_index, who });
+                    Self::deposit_event(Event::EvaluationRequested {
+                        idty_index,
+                        who: who.clone(),
+                    });
                     Ok(())
                 },
             )
@@ -470,14 +531,17 @@ pub mod pallet {
         }
 
         /// Set the distance status using IdtyIndex and AccountId
-        pub fn do_set_distance_status(
-            identity: <T as pallet_identity::Config>::IdtyIndex,
-            status: Option<(<T as frame_system::Config>::AccountId, DistanceStatus)>,
-        ) {
-            IdentityDistanceStatus::<T>::set(identity, status.clone());
-            if let Some((_, DistanceStatus::Valid)) = status {
-                T::OnValidDistanceStatus::on_valid_distance_status(identity);
-            }
+        pub fn do_valid_distance_status(idty: <T as pallet_identity::Config>::IdtyIndex) {
+            // expiration session index
+            let expire_on =
+                pallet_session::Pallet::<T>::current_index() + T::ResultExpiration::get();
+            // index valid result
+            ValidEvaluationResult::<T>::set(idty, Some(()));
+            // schedule expiry
+            ValidEvaluationExpireOn::<T>::append(expire_on, idty);
+            // callback
+            T::OnValidDistanceStatus::on_valid_distance_status(idty);
+            // TODO event
         }
     }
 
@@ -499,42 +563,37 @@ pub mod pallet {
                         MedianResult::One(m) => m,
                         MedianResult::Two(m1, m2) => m1 + (m2 - m1) / 2, // Avoid overflow (since max is 1)
                     };
-                    // flag to capture distance status
+                    // distance status criterion
                     let is_distance_status_valid = median >= T::MinAccessibleReferees::get();
-                    // mutate the distance status accordingly
-                    IdentityDistanceStatus::<T>::mutate(idty, |entry| {
-                        if let Some((account_id, status)) = entry.as_mut() {
-                            // result is ok
+
+                    // take requester and perform unreserve or slash
+                    if let Some(requester) = PendingEvaluationRequest::<T>::take(idty) {
                         if is_distance_status_valid {
-                                // unreserve price
                             T::Currency::unreserve(
-                                    account_id,
+                                &requester,
                                 <T as Config>::EvaluationPrice::get(),
                             );
-                                // update distance status
-                                *status = DistanceStatus::Valid;
-                            }
-                            // result is ko
-                            else {
-                                // slash requester
+                        } else {
                             T::Currency::slash_reserved(
-                                    account_id,
+                                &requester,
                                 <T as Config>::EvaluationPrice::get(),
                             );
-                                // update distance status
-                                *status = DistanceStatus::Invalid;
                         }
-                        } // each entry should be Some((_, DistanceStatus::Pending))
-                    });
-                    // consequences of valid status
+                    }
+                    // if evaluation happened without request, it's ok to do nothing
+
+                    // if evaluation is positive, remember it for antispam
+                    // and perform callbacks
                     if is_distance_status_valid {
-                        T::OnValidDistanceStatus::on_valid_distance_status(idty);
+                        Self::do_valid_distance_status(idty);
+                    } else {
+                        // TODO event
+                    }
                 }
-                } else if let Some((account_id, _status)) = IdentityDistanceStatus::<T>::take(idty)
-                {
                 // when no result is published, all funds are unreserved
+                else if let Some(requester) = PendingEvaluationRequest::<T>::take(idty) {
                     <T as Config>::Currency::unreserve(
-                        &account_id,
+                        &requester,
                         <T as Config>::EvaluationPrice::get(),
                     );
                 }

runtime/common/src/providers.rs

@@ -81,16 +81,15 @@ where
     fn is_distance_ok(
         idty_id: &<T as pallet_identity::Config>::IdtyIndex,
     ) -> Result<(), DispatchError> {
-        match pallet_distance::Pallet::<T>::identity_distance_status(idty_id) {
-            Some((_, status)) => match status {
-                pallet_distance::DistanceStatus::Valid => Ok(()),
-                pallet_distance::DistanceStatus::Invalid => Err(pallet_duniter_wot::Error::<T, frame_support::instances::Instance1>::DistanceIsInvalid.into()),
-                pallet_distance::DistanceStatus::Pending => Err(pallet_duniter_wot::Error::<T, frame_support::instances::Instance1>::DistanceEvaluationPending.into()),
-            },
+        match pallet_distance::Pallet::<T>::valid_evaluation_result(idty_id) {
+            Some(()) => Ok(()),
+			None => match pallet_distance::Pallet::<T>::pending_evaluation_request(idty_id) {
+                Some(_) => Err(pallet_duniter_wot::Error::<T, frame_support::instances::Instance1>::DistanceEvaluationPending.into()),
                 None => Err(pallet_duniter_wot::Error::<T, frame_support::instances::Instance1>::DistanceEvaluationNotRequested.into()),
             }
 		}
     }
+}
 
 #[cfg(feature = "runtime-benchmarks")]
 pub struct BenchmarkSetupHandler<T>(PhantomData<T>);
@@ -109,9 +108,8 @@ macro_rules! impl_benchmark_setup_handler {
                 idty_id: &IdtyIndex,
                 account: &<T as frame_system::Config>::AccountId,
             ) -> () {
-                let _ = pallet_distance::Pallet::<T>::do_set_distance_status(
-                    *idty_id,
-                    Some((account.clone(), pallet_distance::DistanceStatus::Valid)),
+                let _ = pallet_distance::Pallet::<T>::do_valid_distance_status(
+                    *idty_id
                 );
             }
             fn add_cert(issuer: &IdtyIndex, receiver: &IdtyIndex) {