Skip to content
Snippets Groups Projects
Commit 1f16c5e0 authored by Cédric Moreau's avatar Cédric Moreau
Browse files

feat(smith-members): smith account can be excluded, not deleted

parent 04e0face
No related branches found
No related tags found
No related merge requests found
Show whitespace changes
Inline Side-by-side
pallets/smith-members/src/lib.rs
+ 77
− 27
View file @ 1f16c5e0
...@@ -56,6 +56,8 @@ pub enum SmithStatus { ...@@ -56,6 +56,8 @@ pub enum SmithStatus {
Pending, Pending,
/// The identity has reached the requirements to become a smith and can now goGoOnline() or invite/certify other smiths /// The identity has reached the requirements to become a smith and can now goGoOnline() or invite/certify other smiths
Smith, Smith,
/// The identity has been removed from the smiths set but is kept to keep track of its certifications
Excluded,
} }
#[frame_support::pallet] #[frame_support::pallet]
...@@ -148,8 +150,7 @@ pub mod pallet { ...@@ -148,8 +150,7 @@ pub mod pallet {
impl<T: Config> GenesisBuild<T> for GenesisConfig<T> { impl<T: Config> GenesisBuild<T> for GenesisConfig<T> {
fn build(&self) { fn build(&self) {
CurrentSession::<T>::put(0); CurrentSession::<T>::put(0);
let mut cert_meta_by_issuer = let mut cert_meta_by_issuer = BTreeMap::<T::IdtyIndex, Vec<T::IdtyIndex>>::new();
BTreeMap::<T::IdtyIndex, SmithCertMeta<T::BlockNumber>>::new();
for (receiver, issuers) in &self.certs_by_receiver { for (receiver, issuers) in &self.certs_by_receiver {
// Forbid self-cert // Forbid self-cert
assert!( assert!(
...@@ -157,18 +158,13 @@ pub mod pallet { ...@@ -157,18 +158,13 @@ pub mod pallet {
"Identity cannot certify it-self." "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()); let mut issuers_: Vec<_> = Vec::with_capacity(issuers.len());
for (issuer, maybe_removable_on) in issuers { for (issuer, maybe_removable_on) in issuers {
// Count issued certs
cert_meta_by_issuer
.entry(*issuer)
.or_insert(vec![])
.push(*receiver);
issuers_.push(*issuer); issuers_.push(*issuer);
} }
...@@ -187,6 +183,7 @@ pub mod pallet { ...@@ -187,6 +183,7 @@ pub mod pallet {
expires_on: Some( expires_on: Some(
CurrentSession::<T>::get() + T::InactivityMaxDuration::get(), CurrentSession::<T>::get() + T::InactivityMaxDuration::get(),
), ),
issued_certs: vec![],
received_certs: issuers_, received_certs: issuers_,
}, },
); );
...@@ -195,6 +192,16 @@ pub mod pallet { ...@@ -195,6 +192,16 @@ pub mod pallet {
receiver, 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 { ...@@ -227,7 +234,7 @@ pub mod pallet {
/// Invitation must not have been accepted yet /// Invitation must not have been accepted yet
AlreadyAcceptedInvitation, AlreadyAcceptedInvitation,
/// Recevier must not be known among smiths /// Recevier must not be known among smiths
ReceveirAlreadyHasSmithStatus, OnlyExcludedCanComeBack,
/// Recevier must have accepted to eventually become a smith /// Recevier must have accepted to eventually become a smith
ReceveirMustAcceptInvitation, ReceveirMustAcceptInvitation,
/// Issuer must be a smith /// Issuer must be a smith
...@@ -292,24 +299,37 @@ impl<T: Config> Pallet<T> { ...@@ -292,24 +299,37 @@ impl<T: Config> Pallet<T> {
issuer_status == SmithStatus::Smith, issuer_status == SmithStatus::Smith,
Error::<T>::CannotInvite Error::<T>::CannotInvite
); );
if let Some(receiver_meta) = Smiths::<T>::get(receiver) {
ensure!( ensure!(
Smiths::<T>::get(receiver).is_none(), receiver_meta.status == SmithStatus::Excluded,
Error::<T>::ReceveirAlreadyHasSmithStatus Error::<T>::OnlyExcludedCanComeBack
); );
}
Ok(().into()) Ok(().into())
} }
fn do_invite_smith(receiver: T::IdtyIndex) { fn do_invite_smith(receiver: T::IdtyIndex) {
let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get(); let new_expires_on = CurrentSession::<T>::get() + T::InactivityMaxDuration::get();
// 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( Smiths::<T>::insert(
receiver, receiver,
SmithMeta { SmithMeta {
status: SmithStatus::Invited, status: SmithStatus::Invited,
expires_on: Some(new_expires_on), expires_on: Some(new_expires_on),
issued_certs: vec![],
received_certs: vec![], received_certs: vec![],
}, },
); );
}
ExpiresOn::<T>::append(new_expires_on, receiver); ExpiresOn::<T>::append(new_expires_on, receiver);
} }
...@@ -356,8 +376,13 @@ impl<T: Config> Pallet<T> { ...@@ -356,8 +376,13 @@ impl<T: Config> Pallet<T> {
} }
fn do_certify_smith(receiver: T::IdtyIndex, issuer: T::IdtyIndex) { 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| { 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.push(issuer);
maybe_smith_meta.received_certs.sort(); maybe_smith_meta.received_certs.sort();
// TODO: "as u32" allowed? // TODO: "as u32" allowed?
...@@ -381,11 +406,36 @@ impl<T: Config> Pallet<T> { ...@@ -381,11 +406,36 @@ impl<T: Config> Pallet<T> {
if let Some(smith_meta) = Smiths::<T>::get(smith) { if let Some(smith_meta) = Smiths::<T>::get(smith) {
if let Some(expires_on) = smith_meta.expires_on { if let Some(expires_on) = smith_meta.expires_on {
if expires_on == at { 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( T::OnSmithDelete::on_smith_delete(
smith, smith,
SmithRemovalReason::OfflineTooLong, SmithRemovalReason::OfflineTooLong,
); );
// TODO: add event? (+ add others too)
} }
} }
} }
......
pallets/smith-members/src/tests.rs
+ 139
− 14
View file @ 1f16c5e0
...@@ -34,10 +34,11 @@ fn process_to_become_a_smith_and_lose_it() { ...@@ -34,10 +34,11 @@ fn process_to_become_a_smith_and_lose_it() {
4 => None, 4 => None,
], ],
2 => btreemap![ 2 => btreemap![
3 => None,
4 => None, 4 => None,
4 => None, ],
5 => None, 3 => btreemap![],
] 4 => btreemap![],
], ],
}) })
.execute_with(|| { .execute_with(|| {
...@@ -55,6 +56,7 @@ fn process_to_become_a_smith_and_lose_it() { ...@@ -55,6 +56,7 @@ fn process_to_become_a_smith_and_lose_it() {
SmithMeta { SmithMeta {
status: SmithStatus::Pending, status: SmithStatus::Pending,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![],
received_certs: vec![], received_certs: vec![],
} }
); );
...@@ -68,6 +70,7 @@ fn process_to_become_a_smith_and_lose_it() { ...@@ -68,6 +70,7 @@ fn process_to_become_a_smith_and_lose_it() {
SmithMeta { SmithMeta {
status: SmithStatus::Pending, status: SmithStatus::Pending,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![],
received_certs: vec![1], received_certs: vec![1],
} }
); );
...@@ -81,6 +84,7 @@ fn process_to_become_a_smith_and_lose_it() { ...@@ -81,6 +84,7 @@ fn process_to_become_a_smith_and_lose_it() {
SmithMeta { SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![],
received_certs: vec![1, 2], received_certs: vec![1, 2],
} }
); );
...@@ -92,9 +96,33 @@ fn process_to_become_a_smith_and_lose_it() { ...@@ -92,9 +96,33 @@ fn process_to_become_a_smith_and_lose_it() {
assert_eq!(Smiths::<Runtime>::get(5).is_some(), true); assert_eq!(Smiths::<Runtime>::get(5).is_some(), true);
// On session 5 no more smiths because of lack of activity // On session 5 no more smiths because of lack of activity
Pallet::<Runtime>::on_new_session(5); Pallet::<Runtime>::on_new_session(5);
assert_eq!(Smiths::<Runtime>::get(1), None); // TODO: noautoremove (remaining certs) assert_eq!(
assert_eq!(Smiths::<Runtime>::get(2), None); // TODO: noautoremove (remaining certs) Smiths::<Runtime>::get(1),
assert_eq!(Smiths::<Runtime>::get(5), None); // TODO: noautoremove (remaining certs) 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() { ...@@ -127,6 +155,7 @@ fn should_have_checks_on_certify() {
SmithMeta { SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![4],
received_certs: vec![2, 3, 4], received_certs: vec![2, 3, 4],
} }
); );
...@@ -135,6 +164,7 @@ fn should_have_checks_on_certify() { ...@@ -135,6 +164,7 @@ fn should_have_checks_on_certify() {
SmithMeta { SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![1, 4],
received_certs: vec![3, 4], received_certs: vec![3, 4],
} }
); );
...@@ -143,6 +173,7 @@ fn should_have_checks_on_certify() { ...@@ -143,6 +173,7 @@ fn should_have_checks_on_certify() {
SmithMeta { SmithMeta {
status: SmithStatus::Pending, status: SmithStatus::Pending,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![1, 2],
received_certs: vec![4], received_certs: vec![4],
} }
); );
...@@ -151,6 +182,7 @@ fn should_have_checks_on_certify() { ...@@ -151,6 +182,7 @@ fn should_have_checks_on_certify() {
SmithMeta { SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![1, 2, 3],
received_certs: vec![1, 2], received_certs: vec![1, 2],
} }
); );
...@@ -179,6 +211,7 @@ fn should_have_checks_on_certify() { ...@@ -179,6 +211,7 @@ fn should_have_checks_on_certify() {
SmithMeta { SmithMeta {
status: SmithStatus::Pending, status: SmithStatus::Pending,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![1, 2],
received_certs: vec![4], received_certs: vec![4],
} }
); );
...@@ -193,6 +226,7 @@ fn should_have_checks_on_certify() { ...@@ -193,6 +226,7 @@ fn should_have_checks_on_certify() {
SmithMeta { SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: Some(5), expires_on: Some(5),
issued_certs: vec![1, 2],
received_certs: vec![1, 4], received_certs: vec![1, 4],
} }
); );
...@@ -211,7 +245,9 @@ fn smith_activity_postpones_expiration() { ...@@ -211,7 +245,9 @@ fn smith_activity_postpones_expiration() {
2 => btreemap![ 2 => btreemap![
3 => None, 3 => None,
4 => None, 4 => None,
] ],
3 => btreemap![],
4 => btreemap![]
], ],
}) })
.execute_with(|| { .execute_with(|| {
...@@ -223,14 +259,24 @@ fn smith_activity_postpones_expiration() { ...@@ -223,14 +259,24 @@ fn smith_activity_postpones_expiration() {
// Smith #2 is online but not #1 // Smith #2 is online but not #1
Pallet::<Runtime>::smith_goes_online(2); 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); 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!( assert_eq!(
Smiths::<Runtime>::get(2), Smiths::<Runtime>::get(2),
Some(SmithMeta { Some(SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: None, expires_on: None,
issued_certs: vec![],
received_certs: vec![3, 4], received_certs: vec![3, 4],
}) })
); );
...@@ -238,29 +284,108 @@ fn smith_activity_postpones_expiration() { ...@@ -238,29 +284,108 @@ fn smith_activity_postpones_expiration() {
// Smith #2 goes offline // Smith #2 goes offline
Pallet::<Runtime>::on_new_session(6); Pallet::<Runtime>::on_new_session(6);
Pallet::<Runtime>::smith_goes_offline(2); Pallet::<Runtime>::smith_goes_offline(2);
assert_eq!(Smiths::<Runtime>::get(1), None);
assert_eq!( assert_eq!(
Smiths::<Runtime>::get(2), Smiths::<Runtime>::get(2),
Some(SmithMeta { Some(SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: Some(11), expires_on: Some(11),
issued_certs: vec![],
received_certs: vec![3, 4], received_certs: vec![3, 4],
}) })
); );
// Still not expired on session 10 // Still not expired on session 10
Pallet::<Runtime>::on_new_session(10); Pallet::<Runtime>::on_new_session(10);
assert_eq!(Smiths::<Runtime>::get(1), None);
assert_eq!( assert_eq!(
Smiths::<Runtime>::get(2), Smiths::<Runtime>::get(2),
Some(SmithMeta { Some(SmithMeta {
status: SmithStatus::Smith, status: SmithStatus::Smith,
expires_on: Some(11), expires_on: Some(11),
issued_certs: vec![],
received_certs: vec![3, 4], received_certs: vec![3, 4],
}) })
); );
// Finally expired on session 11 // But expired on session 11
Pallet::<Runtime>::on_new_session(11); Pallet::<Runtime>::on_new_session(11);
assert_eq!(Smiths::<Runtime>::get(1), None); assert_eq!(
assert_eq!(Smiths::<Runtime>::get(2), None); 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]
})
);
}); });
} }
pallets/smith-members/src/types.rs
+ 3
− 1
View file @ 1f16c5e0
...@@ -30,7 +30,9 @@ pub struct SmithMeta<IdtyIndex> { ...@@ -30,7 +30,9 @@ pub struct SmithMeta<IdtyIndex> {
pub status: SmithStatus, pub status: SmithStatus,
/// the session at which the smith will expire (for lack of validation activity) /// the session at which the smith will expire (for lack of validation activity)
pub expires_on: Option<SessionIndex>, 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>, pub received_certs: Vec<IdtyIndex>,
} }
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment