diff --git a/pallets/distance/src/benchmarking.rs b/pallets/distance/src/benchmarking.rs
index cb7283931f426bd09f6457ad38afadc84f13132b..c389b354c211c700c7bbe702d58f60c86f3b4341 100644
--- a/pallets/distance/src/benchmarking.rs
+++ b/pallets/distance/src/benchmarking.rs
@@ -75,7 +75,7 @@ benchmarks! {
|idty_val| idty_val.as_mut().unwrap().status = pallet_identity::IdtyStatus::Unvalidated);
}: _<T::RuntimeOrigin>(caller_origin.clone(), target)
verify {
- assert!(PendingEvaluationRequest::<T>::get(&target) == Some(caller.clone()), "Request not added");
+ assert!(PendingEvaluationRequest::<T>::get(target) == Some(caller.clone()), "Request not added");
assert_has_event::<T>(Event::<T>::EvaluationRequested { idty_index: target, who: caller }.into());
}
diff --git a/pallets/distance/src/lib.rs b/pallets/distance/src/lib.rs
index 8f0e7df822660fe2fbee8287addddcf96b06e91e..3a0f01ff4c275f9d30e2098feb6187986ae6ac1b 100644
--- a/pallets/distance/src/lib.rs
+++ b/pallets/distance/src/lib.rs
@@ -464,6 +464,8 @@ pub mod pallet {
Pallet::<T>::mutate_current_pool(
pallet_session::CurrentIndex::<T>::get(), // TODO look
|current_pool| {
+ // TODO not needed if called in extrinsics only
+ // since extrinsics are transactional by default
ensure!(
current_pool.evaluations.len() < (MAX_EVALUATIONS_PER_SESSION as usize),
Error::<T>::QueueFull
diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index 0f6bed36a15ec67cb8d171020e6cb08eee4a4074..b0264bc1fc5017cd1299c072edb4a286b49c9168 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -366,23 +366,42 @@ impl<T: Config<I> + pallet_distance::Config, I: 'static>
fn on_valid_distance_status(idty_index: IdtyIndex) {
if let Some(identity) = pallet_identity::Identities::<T>::get(idty_index) {
match identity.status {
- IdtyStatus::Unconfirmed => { /* should not happen */ }
+ IdtyStatus::Unconfirmed | IdtyStatus::Revoked => {
+ // IdtyStatus::Unconfirmed
+ // distance evaluation request should never happen for unconfirmed identity
+ // IdtyStatus::Revoked
+ // the identity can have been revoked during distance evaluation by the oracle
+ }
+
IdtyStatus::Unvalidated | IdtyStatus::NotMember => {
- // ok to fail
- let r = pallet_membership::Pallet::<T, I>::try_claim_membership(idty_index);
- sp_std::if_std! {
- if let Err(e) = r {
- print!("failed to claim identity when distance status was ok: ");
- println!("{:?}", idty_index);
- println!("reason: {:?}", e);
- }
- }
+ // IdtyStatus::Unvalidated
+ // normal scenario for first entry
+ // IdtyStatus::NotMember
+ // normal scenario for re-entry
+ // the following can fail if a certification expired during distance evaluation
+ // otherwise it should succeed
+ let _ = pallet_membership::Pallet::<T, I>::try_claim_membership(idty_index);
+ // sp_std::if_std! {
+ // if let Err(e) = r {
+ // print!("failed to claim identity when distance status was found ok: ");
+ // println!("{:?}", idty_index);
+ // println!("reason: {:?}", e);
+ // }
+ // }
}
IdtyStatus::Member => {
- // ok to fail
+ // IdtyStatus::Member
+ // normal scenario for renewal
+ // should succeed
let _ = pallet_membership::Pallet::<T, I>::try_renew_membership(idty_index);
+ // sp_std::if_std! {
+ // if let Err(e) = r {
+ // print!("failed to renew identity when distance status was found ok: ");
+ // println!("{:?}", idty_index);
+ // println!("reason: {:?}", e);
+ // }
+ // }
}
- IdtyStatus::Revoked => { /* should not happen */ }
}
} else {
// identity was removed before distance status was found
diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs
index 99f9a317af73b19ff1fa545e0b029de266f9fcd3..c098d4bb506e80ecf1f19c1bac6a6c856624f766 100644
--- a/pallets/membership/src/lib.rs
+++ b/pallets/membership/src/lib.rs
@@ -174,6 +174,8 @@ pub mod pallet {
MembershipAlreadyAcquired,
/// Membership not found.
MembershipNotFound,
+ /// Already member, can not claim membership
+ AlreadyMember,
}
// HOOKS //
@@ -260,6 +262,12 @@ pub mod pallet {
/// check that membership can be claimed
pub fn check_allowed_to_claim(idty_id: T::IdtyId) -> Result<(), DispatchError> {
+ // no-op is error
+ ensure!(
+ Membership::<T, I>::get(idty_id).is_none(),
+ Error::<T, I>::AlreadyMember
+ );
+
// enough certifications and distance rule for example
T::CheckMembershipCallAllowed::check_idty_allowed_to_claim_membership(&idty_id)?;
Ok(())
diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index 177efe07df6516a3e63fd68d4747af4ddf55dfaa..573b249f8b83cfcc1c3cc7b8a38ee2ab4e69f6e6 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -310,7 +310,7 @@ fn test_remove_identity() {
});
}
-/// test identity is validated when membership is claimed
+/// test identity is "validated" (= membership is claimed) when distance is evaluated positively
#[test]
fn test_validate_identity_when_claim() {
ExtBuilder::new(1, 3, 4)
@@ -345,11 +345,14 @@ fn test_validate_identity_when_claim() {
5
));
+ // eve request distance evaluation for herself
assert_ok!(Distance::request_distance_evaluation(
frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(),
));
- run_to_block(51); // Pass 2 sessions
+ // Pass 2 sessions
+ run_to_block(51);
+ // simulate an evaluation published by smith Alice
assert_ok!(Distance::force_update_evaluation(
frame_system::RawOrigin::Root.into(),
AccountKeyring::Alice.to_account_id(),
@@ -357,22 +360,28 @@ fn test_validate_identity_when_claim() {
distances: vec![Perbill::one()],
}
));
- run_to_block(76); // Pass 1 session
-
- // eve can claim her membership
- assert_ok!(Membership::claim_membership(
- frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(),
+ run_to_block(75); // Pass 1 session
+ System::assert_has_event(RuntimeEvent::Distance(
+ pallet_distance::Event::EvaluatedValid { idty_index: 5 },
));
+ // eve can not claim her membership manually because it is done automatically
+ assert_noop!(
+ Membership::claim_membership(
+ frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(),
+ ),
+ pallet_membership::Error::<Runtime, pallet_membership::Instance1>::AlreadyMember
+ );
+
+ // println!("{:?}", System::events());
System::assert_has_event(RuntimeEvent::Membership(
pallet_membership::Event::MembershipAdded {
member: 5,
- expire_on: 76
+ expire_on: 75
+ <Runtime as pallet_membership::Config<Instance1>>::MembershipPeriod::get(
),
},
));
- // not possible anymore to validate identity of someone else
});
}
@@ -677,14 +686,18 @@ fn test_ud_claimed_membership_on_and_off() {
},
));
- // alice claims back her membership
+ // alice claims back her membership through distance evaluation
assert_ok!(Distance::force_valid_distance_status(
frame_system::RawOrigin::Root.into(),
1,
));
- assert_ok!(Membership::claim_membership(
- frame_system::RawOrigin::Signed(AccountKeyring::Alice.to_account_id()).into()
- ));
+ // it can not be done manually
+ assert_noop!(
+ Membership::claim_membership(
+ frame_system::RawOrigin::Signed(AccountKeyring::Alice.to_account_id()).into(),
+ ),
+ pallet_membership::Error::<Runtime, pallet_membership::Instance1>::AlreadyMember
+ );
System::assert_has_event(RuntimeEvent::Membership(
pallet_membership::Event::MembershipAdded {
member: 1,
@@ -1119,20 +1132,25 @@ fn test_validate_new_idty_after_few_uds() {
pallet_identity::IdtyName::from("Eve"),
));
- // At next block, Bob should be able to certify and validate the new identity
+ // At next block, Bob should be able to certify the new identity
run_to_block(23);
assert_ok!(Cert::add_cert(
frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into(),
2,
5,
));
+ // valid distance status should trigger identity validation
assert_ok!(Distance::force_valid_distance_status(
frame_system::RawOrigin::Root.into(),
5,
));
- assert_ok!(Membership::claim_membership(
- frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(),
- ));
+ // and it is not possible to call it manually
+ assert_noop!(
+ Membership::claim_membership(
+ frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(),
+ ),
+ pallet_membership::Error::<Runtime, pallet_membership::Instance1>::AlreadyMember
+ );
// The new member should have first_eligible_ud equal to three
assert!(Identity::identity(5).is_some());
@@ -1181,14 +1199,18 @@ fn test_claim_memberhsip_after_few_uds() {
5,
));
- // eve should be able to claim her membership
+ // eve membership should be able to be claimed through distance evaluation
assert_ok!(Distance::force_valid_distance_status(
frame_system::RawOrigin::Root.into(),
5,
));
- assert_ok!(Membership::claim_membership(
- frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(),
- ));
+ // but not manually
+ assert_noop!(
+ Membership::claim_membership(
+ frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into(),
+ ),
+ pallet_membership::Error::<Runtime, pallet_membership::Instance1>::AlreadyMember
+ );
// The new member should have first_eligible_ud equal to three
assert!(Identity::identity(5).is_some());