diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs
index 587e82aa343c51fbc62bf36d61d3755a68d536f4..f4118ee3aed9eb692ef20fecf8cd7dbd22cc3cff 100644
--- a/pallets/authority-members/src/lib.rs
+++ b/pallets/authority-members/src/lib.rs
@@ -575,14 +575,25 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> {
return None;
}
+ // -- handle incoming members
+ // callback when smith is incoming
for member_id in members_ids_to_add.iter() {
T::OnIncomingMember::on_incoming_member(*member_id);
+ }
+ // a single event with all authorities if some
+ if !members_ids_to_add.is_empty() {
Self::deposit_event(Event::IncomingAuthorities {
members: members_ids_to_add.clone(),
});
}
+
+ // -- handle outgoing members
+ // callback when smith is outgoing
for member_id in members_ids_to_del.iter() {
T::OnOutgoingMember::on_outgoing_member(*member_id);
+ }
+ // a single event with all authorities if some
+ if !members_ids_to_del.is_empty() {
Self::deposit_event(Event::OutgoingAuthorities {
members: members_ids_to_del.clone(),
});
diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 35918f55a00283378fd80f27686c8ecf7c6e0845..1ed34c47bf4fc9e8bfe788aa05bee4f19cff9490 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -582,7 +582,7 @@ impl<T: Config> Pallet<T> {
if let Some(smith_meta) = maybe_smith_meta {
// As long as the smith is online, it cannot expire
smith_meta.expires_on = None;
- // FIXME: unschedule old expiry
+ // FIXME: unschedule old expiry (#182)
}
});
}
@@ -592,10 +592,12 @@ impl<T: Config> Pallet<T> {
/// Handle the event when a Smith goes offline.
pub fn on_smith_goes_offline(idty_index: T::IdtyIndex) {
if let Some(smith_meta) = Smiths::<T>::get(idty_index) {
- if smith_meta.expires_on.is_none() {
+ // Smith can go offline after main membership expiry
+ // in this case, there is no scheduled expiry since it is already excluded
+ if smith_meta.status != SmithStatus::Excluded {
Smiths::<T>::mutate(idty_index, |maybe_smith_meta| {
if let Some(smith_meta) = maybe_smith_meta {
- // As long as the smith is online, it cannot expire
+ // schedule expiry
let new_expires_on =
CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get();
smith_meta.expires_on = Some(new_expires_on);
diff --git a/runtime/gdev/Cargo.toml b/runtime/gdev/Cargo.toml
index 3299b812d1622b27fc3fea94ee79aa01471c580d..08fc4435aad8cf4a23a28a6c102e5826216355c4 100644
--- a/runtime/gdev/Cargo.toml
+++ b/runtime/gdev/Cargo.toml
@@ -112,6 +112,7 @@ std = [
"sp-genesis-builder/std",
"sp-inherents/std",
"sp-io/std",
+ "sp-keyring/std",
"sp-membership/std",
"sp-offchain/std",
"sp-runtime/std",
diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index 5ca299b4313ca499edf81756b5a396f23e263fc4..a8b3cb47bb304485811570f2a50c14685ab7b5f5 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -25,6 +25,7 @@ use frame_support::{
use gdev_runtime::*;
use pallet_identity::{RevocationPayload, REVOCATION_PAYLOAD_PREFIX};
use pallet_membership::MembershipRemovalReason;
+use pallet_session::historical::SessionManager;
use pallet_smith_members::{SmithMeta, SmithStatus};
use scale_info::prelude::num::NonZeroU16;
use sp_core::{Encode, Pair};
@@ -1020,6 +1021,93 @@ fn test_smith_process() {
})
}
+// reveal bug from #243
+#[test]
+fn test_expired_smith_has_null_expires_on() {
+ // initial_authorities_len = 2 → Alice and Bob are online
+ // initial_smiths_len = 3 → Charlie is offline Smith
+ // initial_identities_len = 4 → Dave is member but not smith
+ ExtBuilder::new(2, 3, 4).build().execute_with(|| {
+ run_to_block(1);
+
+ // Bob is smith
+ assert_eq!(
+ SmithMembers::smiths(2),
+ Some(pallet_smith_members::SmithMeta {
+ status: SmithStatus::Smith,
+ expires_on: None, // because online
+ issued_certs: vec![1, 3],
+ received_certs: vec![1, 3],
+ })
+ );
+
+ // force Bob to leave by expiring his main WoT membership
+ Membership::do_remove_membership(2, MembershipRemovalReason::System);
+
+ // check events
+ // membership removal
+ System::assert_has_event(RuntimeEvent::Membership(
+ pallet_membership::Event::MembershipRemoved {
+ member: 2,
+ reason: MembershipRemovalReason::System,
+ },
+ ));
+ // smith membership removal
+ System::assert_has_event(RuntimeEvent::SmithMembers(
+ pallet_smith_members::Event::SmithMembershipRemoved { idty_index: 2 },
+ ));
+ System::assert_has_event(RuntimeEvent::AuthorityMembers(
+ pallet_authority_members::Event::MemberRemoved { member: 2 },
+ ));
+ // also events for certifications
+
+ // check state
+ // Bob is not Smith anymore
+ assert_eq!(
+ SmithMembers::smiths(2),
+ Some(pallet_smith_members::SmithMeta {
+ status: SmithStatus::Excluded, // automatically excluded
+ expires_on: None, // because excluded, no expiry is scheduled
+ issued_certs: vec![1, 3],
+ received_certs: vec![], // received certs are deleted
+ })
+ );
+ // Alice smith cert to Bob has been deleted
+ assert_eq!(
+ SmithMembers::smiths(1),
+ Some(pallet_smith_members::SmithMeta {
+ status: SmithStatus::Smith,
+ expires_on: None, // because online
+ issued_certs: vec![3], // cert to Bob has been deleted
+ received_certs: vec![2, 3],
+ })
+ );
+
+ // run to next block
+ run_to_block(2);
+
+ // simulate new session
+ AuthorityMembers::new_session(2);
+ // check event
+ System::assert_has_event(RuntimeEvent::AuthorityMembers(
+ pallet_authority_members::Event::OutgoingAuthorities { members: vec![2] },
+ ));
+
+ // control state is still ok
+ assert_eq!(
+ SmithMembers::smiths(2),
+ Some(pallet_smith_members::SmithMeta {
+ status: SmithStatus::Excluded, // still excluded
+ expires_on: None, // should be still None
+ issued_certs: vec![1, 3],
+ received_certs: vec![],
+ })
+ );
+
+ // println!("{:#?}", System::events()); // with -- --nocapture
+ })
+}
+
/// test new account creation
#[test]
fn test_create_new_account() {