From 1cefacbae0f8d7201727f84a4409e05950d47c24 Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Sun, 17 Dec 2023 14:58:31 +0100
Subject: [PATCH] feat(smith-members): smith account can be excluded, not
deleted
---
pallets/smith-members/src/lib.rs | 104 +++++++++++++++-----
pallets/smith-members/src/tests.rs | 153 ++++++++++++++++++++++++++---
pallets/smith-members/src/types.rs | 4 +-
3 files changed, 219 insertions(+), 42 deletions(-)
diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 131b93d70..3a7cd8bec 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -56,6 +56,8 @@ pub enum SmithStatus {
Pending,
/// The identity has reached the requirements to become a smith and can now goGoOnline() or invite/certify other smiths
Smith,
+ /// The identity has been removed from the smiths set but is kept to keep track of its certifications
+ Excluded,
}
#[frame_support::pallet]
@@ -148,8 +150,7 @@ pub mod pallet {
impl<T: Config> GenesisBuild<T> for GenesisConfig<T> {
fn build(&self) {
CurrentSession::<T>::put(0);
- let mut cert_meta_by_issuer =
- BTreeMap::<T::IdtyIndex, SmithCertMeta<T::BlockNumber>>::new();
+ let mut cert_meta_by_issuer = BTreeMap::<T::IdtyIndex, Vec<T::IdtyIndex>>::new();
for (receiver, issuers) in &self.certs_by_receiver {
// Forbid self-cert
assert!(
@@ -157,18 +158,13 @@ pub mod pallet {
"Identity cannot certify it-self."
);
- // We should insert cert_meta for receivers that have not issued any cert.
- cert_meta_by_issuer
- .entry(*receiver)
- .or_insert(SmithCertMeta {
- issued_count: 0,
- next_issuable_on: sp_runtime::traits::Zero::zero(),
- received_count: 0,
- })
- .received_count = issuers.len() as u32;
-
let mut issuers_: Vec<_> = Vec::with_capacity(issuers.len());
for (issuer, maybe_removable_on) in issuers {
+ // Count issued certs
+ cert_meta_by_issuer
+ .entry(*issuer)
+ .or_insert(vec![])
+ .push(*receiver);
issuers_.push(*issuer);
}
@@ -187,6 +183,7 @@ pub mod pallet {
expires_on: Some(
CurrentSession::<T>::get() + T::InactivityMaxDuration::get(),
),
+ issued_certs: vec![],
received_certs: issuers_,
},
);
@@ -195,6 +192,16 @@ pub mod pallet {
receiver,
);
}
+
+ for (issuer, issued_certs) in cert_meta_by_issuer {
+ // Write CertsByIssuer
+ Smiths::<T>::mutate(issuer, |maybe_smith_meta| {
+ let maybe_smith_meta = maybe_smith_meta
+ .as_mut()
+ .expect("issuers must have received certs as well");
+ maybe_smith_meta.issued_certs = issued_certs;
+ });
+ }
}
}
@@ -227,7 +234,7 @@ pub mod pallet {
/// Invitation must not have been accepted yet
AlreadyAcceptedInvitation,
/// Recevier must not be known among smiths
- ReceveirAlreadyHasSmithStatus,
+ OnlyExcludedCanComeBack,
/// Recevier must have accepted to eventually become a smith
ReceveirMustAcceptInvitation,
/// Issuer must be a smith
@@ -292,24 +299,37 @@ impl<T: Config> Pallet<T> {
issuer_status == SmithStatus::Smith,
Error::<T>::CannotInvite
);
- ensure!(
- Smiths::<T>::get(receiver).is_none(),
- Error::<T>::ReceveirAlreadyHasSmithStatus
- );
+ if let Some(receiver_meta) = Smiths::<T>::get(receiver) {
+ ensure!(
+ receiver_meta.status == SmithStatus::Excluded,
+ Error::<T>::OnlyExcludedCanComeBack
+ );
+ }
Ok(().into())
}
fn do_invite_smith(receiver: T::IdtyIndex) {
let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get();
- Smiths::<T>::insert(
- receiver,
- SmithMeta {
- status: SmithStatus::Invited,
- expires_on: Some(new_expires_on),
- received_certs: vec![],
- },
- );
+ // TODO: another way to write this?
+ if Smiths::<T>::get(receiver).is_some() {
+ Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
+ let maybe_smith_meta = maybe_smith_meta.as_mut().expect("checked earlier");
+ maybe_smith_meta.status = SmithStatus::Invited;
+ maybe_smith_meta.expires_on = Some(new_expires_on);
+ maybe_smith_meta.received_certs = vec![];
+ });
+ } else {
+ Smiths::<T>::insert(
+ receiver,
+ SmithMeta {
+ status: SmithStatus::Invited,
+ expires_on: Some(new_expires_on),
+ issued_certs: vec![],
+ received_certs: vec![],
+ },
+ );
+ }
ExpiresOn::<T>::append(new_expires_on, receiver);
}
@@ -356,8 +376,13 @@ impl<T: Config> Pallet<T> {
}
fn do_certify_smith(receiver: T::IdtyIndex, issuer: T::IdtyIndex) {
+ Smiths::<T>::mutate(issuer, |maybe_smith_meta| {
+ let maybe_smith_meta = maybe_smith_meta.as_mut().expect("issuer checked earlier");
+ maybe_smith_meta.issued_certs.push(receiver);
+ maybe_smith_meta.issued_certs.sort();
+ });
Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
- let maybe_smith_meta = maybe_smith_meta.as_mut().expect("status checked earlier");
+ let maybe_smith_meta = maybe_smith_meta.as_mut().expect("receiver checked earlier");
maybe_smith_meta.received_certs.push(issuer);
maybe_smith_meta.received_certs.sort();
// TODO: "as u32" allowed?
@@ -381,11 +406,36 @@ impl<T: Config> Pallet<T> {
if let Some(smith_meta) = Smiths::<T>::get(smith) {
if let Some(expires_on) = smith_meta.expires_on {
if expires_on == at {
- Smiths::<T>::remove(smith);
+ let mut lost_certs = vec![];
+ Smiths::<T>::mutate(smith, |maybe_smith_meta| {
+ let maybe_smith_meta =
+ maybe_smith_meta.as_mut().expect("checked earlier");
+ maybe_smith_meta.expires_on = None;
+ maybe_smith_meta.status = SmithStatus::Excluded;
+ for cert in &maybe_smith_meta.received_certs {
+ lost_certs.push(cert.clone());
+ }
+ maybe_smith_meta.received_certs = vec![];
+ // N.B.: the issued certs are kept in case the smith joins back
+ });
+ // We remove the lost certs from their issuer's stock
+ for lost_cert in lost_certs {
+ Smiths::<T>::mutate(lost_cert, |maybe_smith_meta| {
+ let maybe_smith_meta =
+ maybe_smith_meta.as_mut().expect("checked earlier");
+ if let Ok(index) =
+ maybe_smith_meta.issued_certs.binary_search(&smith)
+ {
+ maybe_smith_meta.issued_certs.remove(index);
+ }
+ });
+ }
+ // Deletion done
T::OnSmithDelete::on_smith_delete(
smith,
SmithRemovalReason::OfflineTooLong,
);
+ // TODO: add event? (+ add others too)
}
}
}
diff --git a/pallets/smith-members/src/tests.rs b/pallets/smith-members/src/tests.rs
index 3f2215510..42b417077 100644
--- a/pallets/smith-members/src/tests.rs
+++ b/pallets/smith-members/src/tests.rs
@@ -34,10 +34,11 @@ fn process_to_become_a_smith_and_lose_it() {
4 => None,
],
2 => btreemap![
+ 3 => None,
4 => None,
- 4 => None,
- 5 => None,
- ]
+ ],
+ 3 => btreemap![],
+ 4 => btreemap![],
],
})
.execute_with(|| {
@@ -55,6 +56,7 @@ fn process_to_become_a_smith_and_lose_it() {
SmithMeta {
status: SmithStatus::Pending,
expires_on: Some(5),
+ issued_certs: vec![],
received_certs: vec![],
}
);
@@ -68,6 +70,7 @@ fn process_to_become_a_smith_and_lose_it() {
SmithMeta {
status: SmithStatus::Pending,
expires_on: Some(5),
+ issued_certs: vec![],
received_certs: vec![1],
}
);
@@ -81,6 +84,7 @@ fn process_to_become_a_smith_and_lose_it() {
SmithMeta {
status: SmithStatus::Smith,
expires_on: Some(5),
+ issued_certs: vec![],
received_certs: vec![1, 2],
}
);
@@ -92,9 +96,33 @@ fn process_to_become_a_smith_and_lose_it() {
assert_eq!(Smiths::<Runtime>::get(5).is_some(), true);
// On session 5 no more smiths because of lack of activity
Pallet::<Runtime>::on_new_session(5);
- assert_eq!(Smiths::<Runtime>::get(1), None); // TODO: noautoremove (remaining certs)
- assert_eq!(Smiths::<Runtime>::get(2), None); // TODO: noautoremove (remaining certs)
- assert_eq!(Smiths::<Runtime>::get(5), None); // TODO: noautoremove (remaining certs)
+ assert_eq!(
+ Smiths::<Runtime>::get(1),
+ Some(SmithMeta {
+ status: SmithStatus::Excluded,
+ expires_on: None,
+ issued_certs: vec![],
+ received_certs: vec![]
+ })
+ );
+ assert_eq!(
+ Smiths::<Runtime>::get(2),
+ Some(SmithMeta {
+ status: SmithStatus::Excluded,
+ expires_on: None,
+ issued_certs: vec![],
+ received_certs: vec![]
+ })
+ );
+ assert_eq!(
+ Smiths::<Runtime>::get(5),
+ Some(SmithMeta {
+ status: SmithStatus::Excluded,
+ expires_on: None,
+ issued_certs: vec![],
+ received_certs: vec![]
+ })
+ );
});
}
@@ -127,6 +155,7 @@ fn should_have_checks_on_certify() {
SmithMeta {
status: SmithStatus::Smith,
expires_on: Some(5),
+ issued_certs: vec![4],
received_certs: vec![2, 3, 4],
}
);
@@ -135,6 +164,7 @@ fn should_have_checks_on_certify() {
SmithMeta {
status: SmithStatus::Smith,
expires_on: Some(5),
+ issued_certs: vec![1, 4],
received_certs: vec![3, 4],
}
);
@@ -143,6 +173,7 @@ fn should_have_checks_on_certify() {
SmithMeta {
status: SmithStatus::Pending,
expires_on: Some(5),
+ issued_certs: vec![1, 2],
received_certs: vec![4],
}
);
@@ -151,6 +182,7 @@ fn should_have_checks_on_certify() {
SmithMeta {
status: SmithStatus::Smith,
expires_on: Some(5),
+ issued_certs: vec![1, 2, 3],
received_certs: vec![1, 2],
}
);
@@ -179,6 +211,7 @@ fn should_have_checks_on_certify() {
SmithMeta {
status: SmithStatus::Pending,
expires_on: Some(5),
+ issued_certs: vec![1, 2],
received_certs: vec![4],
}
);
@@ -193,6 +226,7 @@ fn should_have_checks_on_certify() {
SmithMeta {
status: SmithStatus::Smith,
expires_on: Some(5),
+ issued_certs: vec![1, 2],
received_certs: vec![1, 4],
}
);
@@ -211,7 +245,9 @@ fn smith_activity_postpones_expiration() {
2 => btreemap![
3 => None,
4 => None,
- ]
+ ],
+ 3 => btreemap![],
+ 4 => btreemap![]
],
})
.execute_with(|| {
@@ -223,14 +259,24 @@ fn smith_activity_postpones_expiration() {
// Smith #2 is online but not #1
Pallet::<Runtime>::smith_goes_online(2);
- // On session 5 no more smiths because of lack of activity
+ // On session 5: exclusion for lack of activity
Pallet::<Runtime>::on_new_session(5);
- assert_eq!(Smiths::<Runtime>::get(1), None);
+ assert_eq!(
+ Smiths::<Runtime>::get(1),
+ Some(SmithMeta {
+ status: SmithStatus::Excluded,
+ expires_on: None,
+ issued_certs: vec![],
+ received_certs: vec![]
+ })
+ );
+ // issued_certs is empty because #1 was excluded
assert_eq!(
Smiths::<Runtime>::get(2),
Some(SmithMeta {
status: SmithStatus::Smith,
expires_on: None,
+ issued_certs: vec![],
received_certs: vec![3, 4],
})
);
@@ -238,29 +284,108 @@ fn smith_activity_postpones_expiration() {
// Smith #2 goes offline
Pallet::<Runtime>::on_new_session(6);
Pallet::<Runtime>::smith_goes_offline(2);
- assert_eq!(Smiths::<Runtime>::get(1), None);
assert_eq!(
Smiths::<Runtime>::get(2),
Some(SmithMeta {
status: SmithStatus::Smith,
expires_on: Some(11),
+ issued_certs: vec![],
received_certs: vec![3, 4],
})
);
// Still not expired on session 10
Pallet::<Runtime>::on_new_session(10);
- assert_eq!(Smiths::<Runtime>::get(1), None);
assert_eq!(
Smiths::<Runtime>::get(2),
Some(SmithMeta {
status: SmithStatus::Smith,
expires_on: Some(11),
+ issued_certs: vec![],
received_certs: vec![3, 4],
})
);
- // Finally expired on session 11
+ // But expired on session 11
Pallet::<Runtime>::on_new_session(11);
- assert_eq!(Smiths::<Runtime>::get(1), None);
- assert_eq!(Smiths::<Runtime>::get(2), None);
+ assert_eq!(
+ Smiths::<Runtime>::get(1),
+ Some(SmithMeta {
+ status: SmithStatus::Excluded,
+ expires_on: None,
+ issued_certs: vec![],
+ received_certs: vec![]
+ })
+ );
+ assert_eq!(
+ Smiths::<Runtime>::get(2),
+ Some(SmithMeta {
+ status: SmithStatus::Excluded,
+ expires_on: None,
+ issued_certs: vec![],
+ received_certs: vec![]
+ })
+ );
+ });
+}
+
+#[test]
+fn smith_coming_back_recovers_its_issued_certs() {
+ new_test_ext(GenesisConfig {
+ certs_by_receiver: btreemap![
+ 1 => btreemap![
+ 2 => None,
+ 3 => None,
+ 4 => None,
+ ],
+ 2 => btreemap![
+ 3 => None,
+ 4 => None,
+ ],
+ 3 => btreemap![
+ 4 => None,
+ 1 => None,
+ ],
+ 4 => btreemap![]
+ ],
+ })
+ .execute_with(|| {
+ // Not activity for Smith #2
+ Pallet::<Runtime>::smith_goes_online(1);
+ Pallet::<Runtime>::smith_goes_online(3);
+ Pallet::<Runtime>::smith_goes_online(4);
+ // Smith #2 gets excluded
+ Pallet::<Runtime>::on_new_session(5);
+ // The issued certs are preserved
+ assert_eq!(
+ Smiths::<Runtime>::get(2),
+ Some(SmithMeta {
+ status: SmithStatus::Excluded,
+ expires_on: None,
+ issued_certs: vec![1],
+ received_certs: vec![]
+ })
+ );
+ // Smith #2 comes back
+ assert_ok!(Pallet::<Runtime>::invite_smith(RuntimeOrigin::signed(1), 2));
+ assert_ok!(Pallet::<Runtime>::accept_invitation(RuntimeOrigin::signed(
+ 2
+ )));
+ assert_ok!(Pallet::<Runtime>::certify_smith(
+ RuntimeOrigin::signed(1),
+ 2
+ ));
+ assert_ok!(Pallet::<Runtime>::certify_smith(
+ RuntimeOrigin::signed(3),
+ 2
+ ));
+ // Smith #2 is back with its issued certs recovered, but not its received certs
+ assert_eq!(
+ Smiths::<Runtime>::get(2),
+ Some(SmithMeta {
+ status: SmithStatus::Smith,
+ expires_on: Some(10),
+ issued_certs: vec![1],
+ received_certs: vec![1, 3]
+ })
+ );
});
}
diff --git a/pallets/smith-members/src/types.rs b/pallets/smith-members/src/types.rs
index 2242b81a6..a6591f0e3 100644
--- a/pallets/smith-members/src/types.rs
+++ b/pallets/smith-members/src/types.rs
@@ -30,7 +30,9 @@ pub struct SmithMeta<IdtyIndex> {
pub status: SmithStatus,
/// the session at which the smith will expire (for lack of validation activity)
pub expires_on: Option<SessionIndex>,
- /// the recertifications received from other smiths
+ /// the certifications issued to other smiths
+ pub issued_certs: Vec<IdtyIndex>,
+ /// the certifications received from other smiths
pub received_certs: Vec<IdtyIndex>,
}
--
GitLab