diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index d9214f14fcc5a2bf445329ed257ad2536232b528..8927b0952f236effb9a2c36c061b62c5d39bb0b8 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -35,6 +35,7 @@ use traits::*;
use frame_support::pallet_prelude::*;
use pallet_certification::traits::SetNextIssuableOn;
use pallet_identity::{IdtyEvent, IdtyStatus};
+use pallet_membership::MembershipRemovalReason;
use sp_runtime::traits::IsMember;
type IdtyIndex = u32;
@@ -176,7 +177,7 @@ where
// implement cert call checks
impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<IdtyIndex>
for Pallet<T, I>
-// TODO add the following where clause once checks can be done on pallet instance
+// TODO (#136) add the following where clause once checks can be done on pallet instance
// where
// T: pallet_membership::Config<I>,
{
@@ -201,9 +202,10 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id
// 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
+ // able to receive cert
+ IdtyStatus::Unvalidated | IdtyStatus::Member | IdtyStatus::NotMember => {}
IdtyStatus::Unconfirmed => return Err(Error::<T, I>::CertToUnconfirmed.into()),
- IdtyStatus::Revoked => return Err(Error::<T, I>::CertToRevoked.into()), // TODO more self-explanatory error message
+ IdtyStatus::Revoked => return Err(Error::<T, I>::CertToRevoked.into()),
};
} else {
return Err(Error::<T, I>::IdtyNotFound.into());
@@ -283,19 +285,33 @@ impl<T: Config<I>, I: 'static> pallet_identity::traits::OnIdtyChange<T> for Pall
}
// TODO distinguish if identity is revoked (keep it) and if identity is removed (also remove certs)
IdtyEvent::Removed { status } => {
- if *status != IdtyStatus::Revoked {
- if let Err(e) =
- <pallet_certification::Pallet<T, I>>::remove_all_certs_received_by(
- frame_system::Origin::<T>::Root.into(),
- idty_index,
- )
- {
+ // try remove membership in any case
+ <pallet_membership::Pallet<T, I>>::do_remove_membership(
+ idty_index,
+ MembershipRemovalReason::Revoked,
+ );
+
+ // only remove certs if identity is unvalidated
+ match status {
+ IdtyStatus::Unconfirmed | IdtyStatus::Unvalidated => {
+ if let Err(e) =
+ <pallet_certification::Pallet<T, I>>::remove_all_certs_received_by(
+ frame_system::Origin::<T>::Root.into(),
+ idty_index,
+ )
+ {
+ sp_std::if_std! {
+ println!("fail to remove certs received by some idty: {:?}", e)
+ }
+ }
+ }
+ IdtyStatus::Revoked => {}
+ IdtyStatus::Member | IdtyStatus::NotMember => {
sp_std::if_std! {
- println!("fail to remove certs received by some idty: {:?}", e)
+ println!("removed non-revoked identity: {:?}", idty_index);
}
}
}
- // TODO make sure a non-revoked identity can not be removed
}
}
}
@@ -331,12 +347,10 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::OnRemovedCert<IdtyI
&& pallet_membership::Pallet::<T, I>::is_member(&receiver)
{
// expire receiver membership
- // it gives them a bit of time to get back enough certs
- if let Err(e) = <pallet_membership::Pallet<T, I>>::force_expire_membership(receiver) {
- sp_std::if_std! {
- println!("fail to expire membership: {:?}", e)
- }
- }
+ <pallet_membership::Pallet<T, I>>::do_remove_membership(
+ receiver,
+ MembershipRemovalReason::NotEnoughCerts,
+ )
}
}
}
diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs
index 05f44d7bf88a7c86dd3376002a9d8b72bfe9fce9..06d26386841923b4c1369ffb2ee691c9caf5afa1 100644
--- a/pallets/duniter-wot/src/mock.rs
+++ b/pallets/duniter-wot/src/mock.rs
@@ -135,7 +135,7 @@ impl pallet_identity::Config for Test {
type AccountLinker = ();
type Signer = UintAuthorityId;
type Signature = TestSignature;
- type OnIdtyChange = DuniterWot;
+ type OnIdtyChange = (DuniterWot, SmithSubWot);
type RemoveIdentityConsumers = ();
type RuntimeEvent = RuntimeEvent;
type WeightInfo = ();
diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs
index 3950835ae783efd8ae43fefd0fde03fa7a665163..f973097c62deb4fd653c2029d35a5fed6e49a7ef 100644
--- a/pallets/duniter-wot/src/tests.rs
+++ b/pallets/duniter-wot/src/tests.rs
@@ -108,7 +108,7 @@ fn test_smith_certs_expirations_should_expire_smith_membership() {
System::assert_has_event(RuntimeEvent::SmithMembership(
pallet_membership::Event::MembershipRemoved {
member: 1,
- reason: MembershipRemovalReason::Expired,
+ reason: MembershipRemovalReason::NotEnoughCerts,
},
));
});
@@ -139,9 +139,9 @@ fn test_smith_member_cant_change_its_idty_address() {
});
}
-/// members of the smith subwot can not remove their identity
+/// members of the smith subwot can revoke their identity
#[test]
-fn test_smith_member_cant_revoke_its_idty() {
+fn test_smith_member_can_revoke_its_idty() {
new_test_ext(5, 3).execute_with(|| {
run_to_block(2);
@@ -151,15 +151,26 @@ fn test_smith_member_cant_revoke_its_idty() {
};
// Identity 3 can't change it's address
- assert_noop!(
- Identity::revoke_identity(
- RuntimeOrigin::signed(3),
- 3,
- 3,
- TestSignature(3, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode())
- ),
- pallet_duniter_wot::Error::<Test, Instance2>::NotAllowedToRemoveIdty
- );
+ assert_ok!(Identity::revoke_identity(
+ RuntimeOrigin::signed(3),
+ 3,
+ 3,
+ TestSignature(3, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode())
+ ));
+ // membership should be removed
+ System::assert_has_event(RuntimeEvent::Membership(
+ pallet_membership::Event::MembershipRemoved {
+ member: 3,
+ reason: pallet_membership::MembershipRemovalReason::Revoked,
+ },
+ ));
+ // smith membership should be removed as well
+ System::assert_has_event(RuntimeEvent::SmithMembership(
+ pallet_membership::Event::MembershipRemoved {
+ member: 3,
+ reason: pallet_membership::MembershipRemovalReason::Revoked,
+ },
+ ));
});
}
@@ -278,26 +289,37 @@ fn test_revoke_idty() {
new_test_ext(5, 1).execute_with(|| {
run_to_block(2);
- // Alice identity can not be revoked because she's smith
- assert_noop!(
- Identity::revoke_identity(
- RuntimeOrigin::signed(1),
- 1,
+ // Alice identity can be revoked
+ assert_ok!(Identity::revoke_identity(
+ RuntimeOrigin::signed(1),
+ 1,
+ 1,
+ TestSignature(
1,
- TestSignature(
- 1,
- (
- REVOCATION_PAYLOAD_PREFIX,
- RevocationPayload {
- idty_index: 1u32,
- genesis_hash: System::block_hash(0),
- }
- )
- .encode()
+ (
+ REVOCATION_PAYLOAD_PREFIX,
+ RevocationPayload {
+ idty_index: 1u32,
+ genesis_hash: System::block_hash(0),
+ }
)
- ),
- pallet_duniter_wot::Error::<Test, Instance2>::NotAllowedToRemoveIdty
- );
+ .encode()
+ )
+ ));
+ // her membership should be removed
+ System::assert_has_event(RuntimeEvent::Membership(
+ pallet_membership::Event::MembershipRemoved {
+ member: 1,
+ reason: pallet_membership::MembershipRemovalReason::Revoked,
+ },
+ ));
+ // and her smith membership should be removed as well
+ System::assert_has_event(RuntimeEvent::SmithMembership(
+ pallet_membership::Event::MembershipRemoved {
+ member: 1,
+ reason: pallet_membership::MembershipRemovalReason::Revoked,
+ },
+ ));
// Anyone should be able to submit Bob revocation certificate
assert_ok!(Identity::revoke_identity(
@@ -364,8 +386,8 @@ fn test_idty_membership_expire() {
status: IdtyStatus::NotMember,
})
);
- // TODO check that identity is added to auto-revoke list (currently IdentityChangeSchedule)
- // and check the revocation event is there
+ // check that identity is added to auto-revoke list (currently IdentityChangeSchedule)
+ assert_eq!(Identity::next_scheduled(14), vec!(3));
run_to_block(14);
assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(1)));
// Charlie's identity should be auto-revoked at block #11 (8 + 3)
@@ -473,7 +495,7 @@ fn test_certification_expire() {
System::assert_has_event(RuntimeEvent::SmithMembership(
pallet_membership::Event::MembershipRemoved {
member: 1,
- reason: MembershipRemovalReason::Expired,
+ reason: MembershipRemovalReason::NotEnoughCerts,
},
));
@@ -497,7 +519,7 @@ fn test_certification_expire() {
System::assert_has_event(RuntimeEvent::Membership(
pallet_membership::Event::MembershipRemoved {
member: 1,
- reason: MembershipRemovalReason::Expired,
+ reason: MembershipRemovalReason::NotEnoughCerts,
},
));
@@ -579,7 +601,7 @@ fn test_cert_can_not_be_issued() {
System::assert_has_event(RuntimeEvent::SmithMembership(
pallet_membership::Event::MembershipRemoved {
member: 1,
- reason: MembershipRemovalReason::Expired,
+ reason: MembershipRemovalReason::NotEnoughCerts,
},
));
@@ -627,7 +649,7 @@ fn test_cert_can_not_be_issued() {
System::assert_has_event(RuntimeEvent::Membership(
pallet_membership::Event::MembershipRemoved {
member: 1,
- reason: MembershipRemovalReason::Expired,
+ reason: MembershipRemovalReason::NotEnoughCerts,
}, // pending membership expires at 23
));
diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs
index 0de04edab3e642fc90605826c895f016d0cff937..ddf3d0121758633a40f67db42fa8436c7c3f7d82 100644
--- a/pallets/identity/src/lib.rs
+++ b/pallets/identity/src/lib.rs
@@ -739,6 +739,12 @@ pub mod pallet {
);
Self::deposit_event(Event::IdtyRevoked { idty_index, reason });
+ T::OnIdtyChange::on_idty_change(
+ idty_index,
+ &IdtyEvent::Removed {
+ status: IdtyStatus::Revoked,
+ },
+ );
return T::WeightInfo::do_remove_identity();
// TODO benchmark this
// return T::WeightInfo::do_revoke_identity();
diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs
index 53292e3f7f77807df79a3d47222b21de4d8dd386..493e836d2442940a4855014ca4e21f59f152d6a2 100644
--- a/pallets/membership/src/lib.rs
+++ b/pallets/membership/src/lib.rs
@@ -56,8 +56,14 @@ impl<IdtyId, AccountId> SetupBenchmark<IdtyId, AccountId> for () {
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)]
pub enum MembershipRemovalReason {
+ // reach end of life
Expired,
+ // was explicitly revoked
Revoked,
+ // received certs count passed below threshold
+ NotEnoughCerts,
+ // system reasons (consumers, authority members, or root)
+ System,
}
#[frame_support::pallet]
@@ -155,7 +161,6 @@ pub mod pallet {
MembershipRemoved {
member: T::IdtyId,
reason: MembershipRemovalReason,
- // TODO maybe add auto-revocation delay?
},
}
@@ -199,7 +204,7 @@ pub mod pallet {
let idty_id = Self::get_idty_id(origin)?;
Self::check_allowed_to_claim(idty_id)?;
- Self::do_claim_membership(idty_id);
+ Self::do_add_membership(idty_id);
Ok(().into())
}
@@ -231,7 +236,7 @@ pub mod pallet {
let idty_id = Self::get_idty_id(origin)?;
// Apply phase
- Self::do_revoke_membership(idty_id);
+ Self::do_remove_membership(idty_id, MembershipRemovalReason::Revoked);
Ok(().into())
}
@@ -240,25 +245,6 @@ pub mod pallet {
// INTERNAL FUNCTIONS //
impl<T: Config<I>, I: 'static> Pallet<T, I> {
- /// force expire membership
- pub fn force_expire_membership(idty_id: T::IdtyId) -> DispatchResultWithPostInfo {
- Self::do_expire_membership(idty_id);
- Ok(().into())
- }
-
- /// try to claim membership if not already done
- pub fn try_claim_membership(idty_id: T::IdtyId) {
- if Self::check_allowed_to_claim(idty_id).is_ok() {
- Self::do_claim_membership(idty_id);
- }
- // else { should not try to claim membership if not allowed}
- }
-
- /// force revoke membership
- pub fn force_revoke_membership(idty_id: T::IdtyId) {
- Self::do_revoke_membership(idty_id);
- }
-
/// unschedule membership expiry
fn unschedule_membership_expiry(idty_id: T::IdtyId, block_number: T::BlockNumber) {
let mut scheduled = MembershipsExpireOn::<T, I>::get(block_number);
@@ -288,38 +274,25 @@ pub mod pallet {
Ok(())
}
- /// perform membership claim
- fn do_claim_membership(idty_id: T::IdtyId) {
+ /// perform membership addition
+ fn do_add_membership(idty_id: T::IdtyId) {
Self::insert_membership_and_schedule_expiry(idty_id);
T::OnEvent::on_event(&sp_membership::Event::MembershipAdded(idty_id));
}
- /// perform membership revokation
- fn do_revoke_membership(idty_id: T::IdtyId) {
+ /// perform membership removal
+ pub fn do_remove_membership(idty_id: T::IdtyId, reason: MembershipRemovalReason) {
if let Some(membership_data) = Membership::<T, I>::take(idty_id) {
Self::unschedule_membership_expiry(idty_id, membership_data.expire_on);
Self::deposit_event(Event::MembershipRemoved {
member: idty_id,
- reason: MembershipRemovalReason::Revoked,
+ reason,
});
T::OnEvent::on_event(&sp_membership::Event::MembershipRemoved(idty_id));
}
}
- /// perform membership expiration
- fn do_expire_membership(idty_id: T::IdtyId) {
- if Membership::<T, I>::take(idty_id).is_some() {
- Self::deposit_event(Event::MembershipRemoved {
- member: idty_id,
- reason: MembershipRemovalReason::Expired,
- });
- T::OnEvent::on_event(&sp_membership::Event::MembershipRemoved(idty_id));
- } // else should not happen
- }
-
/// check the origin and get identity id if valid
- // TODO avoid the concept of idty_id which does not tell the status of the identity
- // maybe call functions using the origin instead and let the conversion happen in an other pallet
fn get_idty_id(origin: OriginFor<T>) -> Result<T::IdtyId, DispatchError> {
if let Ok(RawOrigin::Signed(account_id)) = origin.into() {
T::IdtyIdOf::convert(account_id).ok_or_else(|| Error::<T, I>::IdtyIdNotFound.into())
@@ -327,20 +300,21 @@ pub mod pallet {
Err(BadOrigin.into())
}
}
+
/// perform the membership expiry scheduled at given block
pub fn expire_memberships(block_number: T::BlockNumber) -> Weight {
let mut expired_idty_count = 0u32;
for idty_id in MembershipsExpireOn::<T, I>::take(block_number) {
// remove membership (take)
- Self::do_expire_membership(idty_id);
+ Self::do_remove_membership(idty_id, MembershipRemovalReason::Expired);
expired_idty_count = 0;
}
T::WeightInfo::expire_memberships(expired_idty_count)
}
/// check if identity is member
- pub(super) fn do_is_member(idty_id: &T::IdtyId) -> bool {
+ pub(super) fn is_member(idty_id: &T::IdtyId) -> bool {
Membership::<T, I>::contains_key(idty_id)
}
}
@@ -350,7 +324,7 @@ pub mod pallet {
impl<T: Config<I>, I: 'static> sp_runtime::traits::IsMember<T::IdtyId> for Pallet<T, I> {
fn is_member(idty_id: &T::IdtyId) -> bool {
- Self::do_is_member(idty_id)
+ Self::is_member(idty_id)
}
}
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index f7f297055c4a192653d0693424e6e85b125306a9..0c8fd0b8e090994c8661fb5c080514e04cd41c1a 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -114,6 +114,7 @@ impl<
}
// authority member removal handler
+// TODO refac smith wot
pub struct OnRemovedAuthorityMemberHandler<Runtime>(core::marker::PhantomData<Runtime>);
impl<Runtime> pallet_authority_members::traits::OnRemovedMember<IdtyIndex>
for OnRemovedAuthorityMemberHandler<Runtime>
@@ -122,11 +123,15 @@ where
{
fn on_removed_member(idty_index: IdtyIndex) {
// TODO investigate why we should remove smith membership when removing authority member
- pallet_membership::Pallet::<Runtime, Instance2>::force_revoke_membership(idty_index);
+ pallet_membership::Pallet::<Runtime, Instance2>::do_remove_membership(
+ idty_index,
+ MembershipRemovalReason::System,
+ );
}
}
// identity removal handler
+// TODO check if it is really useful
pub struct RemoveIdentityConsumersImpl<Runtime>(core::marker::PhantomData<Runtime>);
impl<Runtime> pallet_identity::traits::RemoveIdentityConsumers<IdtyIndex>
for RemoveIdentityConsumersImpl<Runtime>
@@ -139,10 +144,16 @@ where
fn remove_idty_consumers(idty_index: IdtyIndex) -> Weight {
// Remove smith member
if pallet_membership::Pallet::<Runtime, Instance2>::is_member(&idty_index) {
- pallet_membership::Pallet::<Runtime, Instance2>::force_revoke_membership(idty_index);
+ pallet_membership::Pallet::<Runtime, Instance2>::do_remove_membership(
+ idty_index,
+ MembershipRemovalReason::System,
+ );
}
// Remove "classic" member
- pallet_membership::Pallet::<Runtime, Instance1>::force_revoke_membership(idty_index);
+ pallet_membership::Pallet::<Runtime, Instance1>::do_remove_membership(
+ idty_index,
+ MembershipRemovalReason::System,
+ );
Weight::zero()
}
diff --git a/runtime/gdev/tests/fixme_tests.rs b/runtime/gdev/tests/fixme_tests.rs
index f0a283762d55a3a5890ebe2a6b4609394f233f6a..9bbdd6a2bbfa34d93b8f256877b6c144e071e5e5 100644
--- a/runtime/gdev/tests/fixme_tests.rs
+++ b/runtime/gdev/tests/fixme_tests.rs
@@ -41,9 +41,10 @@ fn can_still_issue_cert_when_membership_lost() {
ExtBuilder::new(1, 3, 4).build().execute_with(|| {
run_to_block(1);
- // expire Bob membership (could happen for negative distance evaluation for example)
- assert_ok!(Membership::force_expire_membership(
+ // expire Bob membership
+ assert_ok!(Membership::do_remove_membership(
2, // Bob
+ MembershipRemovalReason::System
));
System::assert_has_event(RuntimeEvent::Membership(
pallet_membership::Event::MembershipRemoved {
diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index 7bacdb04786508a5cc0832edd12ab966386ad722..5d74b29d7543f04ac5e880d9df2dc158ce8bd8b7 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -572,7 +572,10 @@ fn test_ud_claimed_membership_on_and_off() {
run_to_block(13);
// alice identity expires
- assert_ok!(Membership::force_expire_membership(1));
+ assert_ok!(Membership::do_remove_membership(
+ 1,
+ MembershipRemovalReason::System
+ ));
System::assert_has_event(RuntimeEvent::UniversalDividend(
pallet_universal_dividend::Event::UdsAutoPaidAtRemoval {
count: 1,