From cb9631d19f2165a6771f34be4d371d527bb05bb4 Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Tue, 19 Dec 2023 12:06:36 +0100
Subject: [PATCH] feat(smith-members): refac error names + fix cert on excluded
---
pallets/smith-members/src/lib.rs | 70 +++++++++++++++-------------
pallets/smith-members/src/tests.rs | 74 ++++++++++++++++++++++++++++--
2 files changed, 108 insertions(+), 36 deletions(-)
diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 3ec870a07..68221c0cf 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -225,30 +225,28 @@ pub mod pallet {
#[pallet::error]
pub enum Error<T> {
- /// Certification issuer has no known identity ID
- UnknownIssuer,
+ /// Issuer of anything (invitation, acceptance, certification) must have an identity ID
+ OriginMustHaveAnIdentity,
/// Issuer must be known as a potential smith
- IssuerHasNoSmithStatus,
+ OriginHasNeverBeenInvited,
/// Invitation is reseverd to smiths
- CannotInvite,
- /// Must have receive invitation to accept it
- NotInvited,
+ InvitationIsASmithPrivilege,
/// Invitation must not have been accepted yet
- AlreadyAcceptedInvitation,
- /// Recevier must not be known among smiths
- OnlyExcludedCanComeBack,
- /// Recevier must have accepted to eventually become a smith
- ReceveirMustAcceptInvitation,
+ InvitationAlreadyAccepted,
+ /// Invitation of an already known smith is forbidden except if it has been excluded
+ InvitationOfNonExcluded,
+ /// Certification cannot be made on someone who has not accepted an invitation
+ CertificationMustBeAgreed,
+ /// Certification cannot be made on excluded
+ CertificationOnExcludedIsForbidden,
/// Issuer must be a smith
- IssuerIsNotASmith,
+ CertificationIsASmithPrivilege,
/// Smith cannot certify itself
- CannotCertifySelf,
+ CertificationOfSelfIsForbidden,
/// Receiver must be invited by another smith
- ReceiverHasNotBeenInvited,
- /// Smith must be a member of the main WoT
- NotAMember,
+ CertificationReceiverMustHaveBeenInvited,
/// A smith has a limited stock of certifications
- TooMuchCertificationsIssued,
+ CertificationStockFullyConsumed,
}
#[pallet::call]
@@ -260,7 +258,8 @@ pub mod pallet {
receiver: T::IdtyIndex,
) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin.clone())?;
- let issuer = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::UnknownIssuer)?;
+ let issuer =
+ T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::OriginMustHaveAnIdentity)?;
Self::check_invite_smith(issuer, receiver)?;
Self::do_invite_smith(issuer, receiver);
Ok(().into())
@@ -270,7 +269,8 @@ pub mod pallet {
#[pallet::weight(1_000_000_000)]
pub fn accept_invitation(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin.clone())?;
- let receiver = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::UnknownIssuer)?;
+ let receiver =
+ T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::OriginMustHaveAnIdentity)?;
Self::check_accept_invitation(receiver)?;
Self::do_accept_invitation(receiver)?;
Ok(().into())
@@ -283,7 +283,8 @@ pub mod pallet {
receiver: T::IdtyIndex,
) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin)?;
- let issuer = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::UnknownIssuer)?;
+ let issuer =
+ T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T>::OriginMustHaveAnIdentity)?;
Self::check_certify_smith(issuer, receiver)?;
Self::do_certify_smith(receiver, issuer);
Ok(().into())
@@ -297,16 +298,16 @@ impl<T: Config> Pallet<T> {
receiver: T::IdtyIndex,
) -> DispatchResultWithPostInfo {
let issuer_status = Smiths::<T>::get(issuer)
- .ok_or(Error::<T>::IssuerHasNoSmithStatus)?
+ .ok_or(Error::<T>::OriginHasNeverBeenInvited)?
.status;
ensure!(
issuer_status == SmithStatus::Smith,
- Error::<T>::CannotInvite
+ Error::<T>::InvitationIsASmithPrivilege
);
if let Some(receiver_meta) = Smiths::<T>::get(receiver) {
ensure!(
receiver_meta.status == SmithStatus::Excluded,
- Error::<T>::OnlyExcludedCanComeBack
+ Error::<T>::InvitationOfNonExcluded
);
}
@@ -343,11 +344,11 @@ impl<T: Config> Pallet<T> {
fn check_accept_invitation(receiver: T::IdtyIndex) -> DispatchResultWithPostInfo {
let pretender_status = Smiths::<T>::get(receiver)
- .ok_or(Error::<T>::NotInvited)?
+ .ok_or(Error::<T>::OriginHasNeverBeenInvited)?
.status;
ensure!(
pretender_status == SmithStatus::Invited,
- Error::<T>::AlreadyAcceptedInvitation
+ Error::<T>::InvitationAlreadyAccepted
);
Ok(().into())
}
@@ -367,23 +368,30 @@ impl<T: Config> Pallet<T> {
issuer: T::IdtyIndex,
receiver: T::IdtyIndex,
) -> DispatchResultWithPostInfo {
- ensure!(issuer != receiver, Error::<T>::CannotCertifySelf);
- let issuer = Smiths::<T>::get(issuer).ok_or(Error::<T>::IssuerHasNoSmithStatus)?;
+ ensure!(
+ issuer != receiver,
+ Error::<T>::CertificationOfSelfIsForbidden
+ );
+ let issuer = Smiths::<T>::get(issuer).ok_or(Error::<T>::OriginHasNeverBeenInvited)?;
ensure!(
issuer.status == SmithStatus::Smith,
- Error::<T>::IssuerIsNotASmith
+ Error::<T>::CertificationIsASmithPrivilege
);
let issued_certs = issuer.issued_certs.len() as u32;
ensure!(
issued_certs < T::MaxByIssuer::get(),
- Error::<T>::TooMuchCertificationsIssued
+ Error::<T>::CertificationStockFullyConsumed
);
let receiver_status = Smiths::<T>::get(receiver)
- .ok_or(Error::<T>::ReceiverHasNotBeenInvited)?
+ .ok_or(Error::<T>::CertificationReceiverMustHaveBeenInvited)?
.status;
ensure!(
receiver_status != SmithStatus::Invited,
- Error::<T>::ReceveirMustAcceptInvitation
+ Error::<T>::CertificationMustBeAgreed
+ );
+ ensure!(
+ receiver_status != SmithStatus::Excluded,
+ Error::<T>::CertificationOnExcludedIsForbidden
);
Ok(().into())
diff --git a/pallets/smith-members/src/tests.rs b/pallets/smith-members/src/tests.rs
index 885567b6c..0d8eaabd1 100644
--- a/pallets/smith-members/src/tests.rs
+++ b/pallets/smith-members/src/tests.rs
@@ -20,6 +20,7 @@ use super::*;
use crate::mock::{new_test_ext, run_to_block, Runtime, RuntimeEvent, RuntimeOrigin, System};
use frame_support::{assert_err, assert_ok};
+use crate::SmithStatus::{Excluded, Invited, Pending, Smith};
#[cfg(test)]
use maplit::btreemap;
use pallet_authority_members::OnNewSession;
@@ -204,19 +205,19 @@ fn should_have_checks_on_certify() {
// Tries all possible errors
assert_err!(
Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(0), 1),
- Error::<Runtime>::IssuerHasNoSmithStatus
+ Error::<Runtime>::OriginHasNeverBeenInvited
);
assert_err!(
Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(1), 1),
- Error::<Runtime>::CannotCertifySelf
+ Error::<Runtime>::CertificationOfSelfIsForbidden
);
assert_err!(
Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(3), 5),
- Error::<Runtime>::IssuerIsNotASmith
+ Error::<Runtime>::CertificationIsASmithPrivilege
);
assert_err!(
Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(1), 6),
- Error::<Runtime>::ReceiverHasNotBeenInvited
+ Error::<Runtime>::CertificationReceiverMustHaveBeenInvited
);
// #3: state before
@@ -396,7 +397,70 @@ fn smith_coming_back_recovers_its_issued_certs() {
// Max stock is reached (3 = 1 recovered + 2 new)
assert_err!(
Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(2), 5),
- Error::<Runtime>::TooMuchCertificationsIssued
+ Error::<Runtime>::CertificationStockFullyConsumed
+ );
+ });
+}
+
+#[test]
+fn certifying_on_different_status() {
+ new_test_ext(GenesisConfig {
+ certs_by_receiver: btreemap![
+ 1 => vec![2, 3, 4],
+ 2 => vec![3, 4],
+ 3 => vec![1, 2],
+ 4 => vec![],
+ ],
+ })
+ .execute_with(|| {
+ // State before
+ assert_eq!(Smiths::<Runtime>::get(5), None);
+ assert_err!(
+ Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(1), 5),
+ Error::<Runtime>::CertificationReceiverMustHaveBeenInvited
+ );
+
+ // After invitation
+ assert_ok!(Pallet::<Runtime>::invite_smith(RuntimeOrigin::signed(1), 5));
+ assert_eq!(Smiths::<Runtime>::get(5).unwrap().status, Invited);
+ assert_err!(
+ Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(1), 5),
+ Error::<Runtime>::CertificationMustBeAgreed
+ );
+
+ // After acceptation
+ assert_ok!(Pallet::<Runtime>::accept_invitation(RuntimeOrigin::signed(
+ 5
+ )));
+ assert_eq!(Smiths::<Runtime>::get(5).unwrap().status, Pending);
+ assert_ok!(Pallet::<Runtime>::certify_smith(
+ RuntimeOrigin::signed(1),
+ 5
+ ));
+ assert_eq!(Smiths::<Runtime>::get(5).unwrap().status, Pending);
+ assert_ok!(Pallet::<Runtime>::certify_smith(
+ RuntimeOrigin::signed(2),
+ 5
+ ));
+ assert_eq!(Smiths::<Runtime>::get(5).unwrap().status, Smith);
+
+ // After being a smith
+ assert_ok!(Pallet::<Runtime>::certify_smith(
+ RuntimeOrigin::signed(3),
+ 5
+ ));
+
+ Pallet::<Runtime>::smith_goes_online(1);
+ Pallet::<Runtime>::smith_goes_online(2);
+ Pallet::<Runtime>::on_new_session(5);
+ assert_eq!(Smiths::<Runtime>::get(1).unwrap().status, Smith);
+ assert_eq!(Smiths::<Runtime>::get(2).unwrap().status, Smith);
+ assert_eq!(Smiths::<Runtime>::get(5).unwrap().status, Excluded);
+
+ // After being excluded
+ assert_err!(
+ Pallet::<Runtime>::certify_smith(RuntimeOrigin::signed(1), 5),
+ Error::<Runtime>::CertificationOnExcludedIsForbidden
);
});
}
--
GitLab