diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index bcd64dafafc59d15314af3ced929c5e2d3fdea98..f4e336c1e4d594b11951cc5e87602ea8feac1426 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -208,7 +208,22 @@ where
impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<IdtyIndex>
for Pallet<T, I>
{
+ // check the following:
+ // - issuer has identity
+ // - issuer identity is validated
+ // - receiver has identity
+ // - receiver identity is confirmed or validated
+ //
+ // /!\ do not check the following:
+ // - receiver has membership
+ // - issuer has membership
+ // this has the following consequences:
+ // - receiver can receive smith certification without having requested membership
+ // - 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
fn check_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> Result<(), DispatchError> {
+ // ensure issuer has validated identity
if let Some(issuer_data) = pallet_identity::Pallet::<T>::identity(issuer) {
ensure!(
issuer_data.status == IdtyStatus::Validated,
@@ -217,6 +232,7 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id
} else {
return Err(Error::<T, I>::IdtyNotFound.into());
}
+ // ensure receiver has confirmed or validated identity
if let Some(receiver_data) = pallet_identity::Pallet::<T>::identity(receiver) {
match receiver_data.status {
IdtyStatus::ConfirmedByOwner | IdtyStatus::Validated => {} // able to receive cert
diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs
index 175f626ae389869c08cf2a9e51d73cc76707633a..e45e390490ca5a3618ffbe2765bf57c36c4ddb9a 100644
--- a/pallets/duniter-wot/src/tests.rs
+++ b/pallets/duniter-wot/src/tests.rs
@@ -516,3 +516,118 @@ fn test_certification_expire() {
));
})
}
+
+/// test some cases where identity should not be able to issue cert
+// - when source or target is not member (sub wot)
+// - when source or target membership is pending (both wot)
+#[test]
+fn test_cert_can_not_be_issued() {
+ new_test_ext(4, 3).execute_with(|| {
+ // smith cert Bob → Alice not renewed
+ // cert Bob → Alice not renewed
+ // --- BLOCK 2 ---
+ run_to_block(2);
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(1), 1, 2)); // +10
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(2), 2, 3)); // +10
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(3), 3, 1)); // +10
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(1), 1, 2)); // +20
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(2), 2, 3)); // +20
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(3), 3, 4)); // +20
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(4), 4, 1)); // +20
+ // --- BLOCK 4 ---
+ run_to_block(4);
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(1), 1, 3)); // +10
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(3), 3, 2)); // +10
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(2), 2, 4)); // +20
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(3), 3, 2)); // +20
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(4), 4, 3)); // +20
+ // --- BLOCK 7 ---
+ run_to_block(7);
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(3), 3, 1)); // +10
+ assert_ok!(SmithMembership::renew_membership(RuntimeOrigin::signed(1))); // +20
+ assert_ok!(SmithMembership::renew_membership(RuntimeOrigin::signed(2))); // +20
+ assert_ok!(SmithMembership::renew_membership(RuntimeOrigin::signed(3))); // +20
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(1))); // + 8
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(2))); // + 8
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(3))); // + 8
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(4))); // + 8
+
+ // smith cert Bob → Alice expires at block 10
+ run_to_block(10);
+ // println!("{:?}", System::events());
+ System::assert_has_event(RuntimeEvent::SmithCert(
+ pallet_certification::Event::RemovedCert {
+ issuer: 2, // Bob
+ issuer_issued_count: 1, // Bob → Charlie only
+ receiver: 1, // Alice
+ receiver_received_count: 1, // Charlie → Alice only
+ expiration: true,
+ },
+ ));
+ // in consequence, since Alice has only 1/2 smith certification remaining, she looses smith membership
+ System::assert_has_event(RuntimeEvent::SmithMembership(
+ pallet_membership::Event::MembershipExpired(1),
+ ));
+
+ run_to_block(11);
+ // /!\ COUNTERINTUITIVE BEHAVIOR
+ // Dave should not be able to receive a smith cert since he did not request smith membership
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(3), 3, 4));
+ // Bob renews his smith certification towards Alice
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(2), 2, 1));
+ // now Alice has enough certs
+ assert_eq!(
+ SmithCert::idty_cert_meta(1),
+ pallet_certification::IdtyCertMeta {
+ issued_count: 2, // → Bob, → Charlie
+ // since Alice got back to min number of received certs to be able to issue cert
+ // her next_issuable_on was updated to block_number (11) + cert_period (2)
+ next_issuable_on: 13, // 11 + 2
+ received_count: 2 // ↠Bob, ↠Charlie
+ }
+ );
+
+ // because Alice did not claim membership, she is not member at this point
+ assert_eq!(SmithMembership::membership(1), None);
+ run_to_block(13);
+ // /!\ COUNTERINTUITIVE BEHAVIOR
+ // Alice is not smith member, she should not be able to issue cert
+ assert_ok!(SmithCert::add_cert(RuntimeOrigin::signed(1), 1, 2));
+
+ run_to_block(14);
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(1))); // + 8
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(2))); // + 8
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(3))); // + 8
+ assert_ok!(Membership::renew_membership(RuntimeOrigin::signed(4))); // + 8
+
+ run_to_block(20);
+ // println!("{:?}", System::events());
+ System::assert_has_event(RuntimeEvent::Cert(
+ pallet_certification::Event::RemovedCert {
+ issuer: 2, // Bob
+ issuer_issued_count: 2, // depends of the order of cert expiration
+ receiver: 1, // Alice
+ receiver_received_count: 2, // depends of the order of cert expiration
+ expiration: true,
+ },
+ ));
+ // other certifications expire, but not Dave → Alice
+ // in consequence, since Alice has only 1/2 certification remaining, she looses membership
+ System::assert_has_event(RuntimeEvent::Membership(
+ pallet_membership::Event::MembershipExpired(1), // pending membership expires at 23
+ ));
+
+ run_to_block(21);
+ // println!("{:?}", System::events());
+ // Charlie certifies Alice so she again has enough certs
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(3), 3, 1));
+ assert_ok!(Cert::add_cert(RuntimeOrigin::signed(4), 4, 1)); // renew
+ // Alice did not claim membership, she is not member
+ // but her cert delay has been reset (→ 23)
+ assert_eq!(Membership::membership(1), None);
+
+ // run_to_block(23);
+ // if identity of alice was not removed because pending for too long
+ // she would have been able to emit a cert without being member
+ })
+}
diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index b7eb8a87f20d99898a799528a1d89820668ffd2e..dc132093015989997dc76a6caf3ff88dda46fc32 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -632,6 +632,42 @@ fn test_remove_smith_identity() {
});
}
+#[test]
+fn test_smith_certification() {
+ // 3 smith (1. alice, 2. bob, 3. charlie)
+ // 4 identities (4. dave)
+ // no identity 5. eve
+ ExtBuilder::new(1, 3, 4).build().execute_with(|| {
+ run_to_block(1);
+
+ // alice can renew smith cert to bob
+ assert_ok!(SmithCert::add_cert(
+ frame_system::RawOrigin::Signed(AccountKeyring::Alice.to_account_id()).into(),
+ 1, // alice
+ 2 // bob
+ ));
+
+ // THIS IS STRANGE BEHAVIOR
+ // bob can add new smith cert to to dave even he did not requested smith membership
+ assert_ok!(SmithCert::add_cert(
+ frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into(),
+ 2, // bob
+ 4 // dave
+ ));
+
+ // charlie can not add new cert to eve (no identity)
+ assert_noop!(
+ SmithCert::add_cert(
+ frame_system::RawOrigin::Signed(AccountKeyring::Charlie.to_account_id()).into(),
+ 3, // charlie
+ 5 // eve
+ ),
+ // SmithSubWot::Error::IdtyNotFound,
+ pallet_duniter_wot::Error::<gdev_runtime::Runtime, pallet_certification::Instance2>::IdtyNotFound,
+ );
+ });
+}
+
/// test create new account with balance lower than existential deposit
// the treasury gets the dust
#[test]