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

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

parent f37886db
No related branches found
No related tags found
No related merge requests found
This commit is part of merge request !217. Comments created here will be created in the context of that merge request.
...@@ -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
); );
ensure!( if let Some(receiver_meta) = Smiths::<T>::get(receiver) {
Smiths::<T>::get(receiver).is_none(), ensure!(
Error::<T>::ReceveirAlreadyHasSmithStatus receiver_meta.status == SmithStatus::Excluded,
); 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();
Smiths::<T>::insert( // TODO: another way to write this?
receiver, if Smiths::<T>::get(receiver).is_some() {
SmithMeta { Smiths::<T>::mutate(receiver, |maybe_smith_meta| {
status: SmithStatus::Invited, let maybe_smith_meta = maybe_smith_meta.as_mut().expect("checked earlier");
expires_on: Some(new_expires_on), maybe_smith_meta.status = SmithStatus::Invited;
received_certs: vec![], 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); 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)
} }
} }
} }
......
...@@ -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]
})
);
}); });
} }
...@@ -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% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment