diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs index a20e0bd04d2221ab3dfe9008bf02a5aff9ab3a68..bf8527957038796b8727699bad624c5cc4b2dea0 100644 --- a/pallets/smith-members/src/lib.rs +++ b/pallets/smith-members/src/lib.rs @@ -430,6 +430,7 @@ impl<T: Config> Pallet<T> { } fn do_certify_smith(receiver: T::IdtyIndex, issuer: T::IdtyIndex) { + // - adds a certification in issuer issued list Smiths::<T>::mutate(issuer, |maybe_smith_meta| { if let Some(smith_meta) = maybe_smith_meta { smith_meta.issued_certs.push(receiver); @@ -438,25 +439,39 @@ impl<T: Config> Pallet<T> { }); Smiths::<T>::mutate(receiver, |maybe_smith_meta| { if let Some(smith_meta) = maybe_smith_meta { + // - adds a certification in receiver received list smith_meta.received_certs.push(issuer); smith_meta.received_certs.sort(); + Self::deposit_event(Event::<T>::SmithCertAdded { receiver, issuer }); + + // - receiving a certification either lead us to Pending or Smith status + let previous_status = smith_meta.status; smith_meta.status = if smith_meta.received_certs.len() >= T::MinCertForMembership::get() as usize { + // - if the number of certification received by the receiver is enough, win the Smith status (or keep it) SmithStatus::Smith } else { + // - otherwise we are (still) a pending smith SmithStatus::Pending }; - // expiry postponed - let new_expires_on = - CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get(); - smith_meta.expires_on = Some(new_expires_on); - Self::deposit_event(Event::<T>::SmithCertAdded { receiver, issuer }); - if smith_meta.status == SmithStatus::Smith { + + if previous_status != SmithStatus::Smith { + // - postpone the expiration: a Pending smith cannot do anything but wait + // this postponement is here to ease the process of becoming a smith + let new_expires_on = + CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get(); + smith_meta.expires_on = Some(new_expires_on); + ExpiresOn::<T>::append(new_expires_on, receiver); + } + + // - if the status is smith but wasn't, notify that smith gained membership + if smith_meta.status == SmithStatus::Smith && previous_status != SmithStatus::Smith + { Self::deposit_event(Event::<T>::SmithMembershipAdded { idty_index: receiver, }); } - // TODO: unschedule old expiry + // TODO: (optimization) unschedule old expiry } }); } diff --git a/pallets/smith-members/src/tests.rs b/pallets/smith-members/src/tests.rs index a6b9f071525f87efbc5a724904bfb49c40d93877..3f30805ea6fba656fe715c527ebe9266db4f970e 100644 --- a/pallets/smith-members/src/tests.rs +++ b/pallets/smith-members/src/tests.rs @@ -187,6 +187,52 @@ fn process_to_become_a_smith_and_lose_it() { }); } +#[test] +fn avoid_multiple_events_for_becoming_smith() { + new_test_ext(GenesisConfig { + initial_smiths: btreemap![ + 1 => (false, vec![2, 3, 4]), + 2 => (false, vec![3, 4]), + 3 => (false, vec![1, 2]), + 4 => (false, vec![]), + ], + }) + .execute_with(|| { + // Go online to be able to invite+certify + Pallet::<Runtime>::on_smith_goes_online(1); + Pallet::<Runtime>::on_smith_goes_online(2); + Pallet::<Runtime>::on_smith_goes_online(3); + // Events cannot be recorded on genesis + run_to_block(1); + // State before + assert_eq!(Smiths::<Runtime>::get(5), None); + // Try to invite + assert_ok!(Pallet::<Runtime>::invite_smith(RuntimeOrigin::signed(1), 5)); + assert_ok!(Pallet::<Runtime>::accept_invitation(RuntimeOrigin::signed( + 5 + ))); + assert_ok!(Pallet::<Runtime>::certify_smith( + RuntimeOrigin::signed(1), + 5 + )); + assert_ok!(Pallet::<Runtime>::certify_smith( + RuntimeOrigin::signed(2), + 5 + )); + System::assert_has_event(RuntimeEvent::Smith( + Event::<Runtime>::SmithMembershipAdded { idty_index: 5 }, + )); + run_to_block(2); + assert_ok!(Pallet::<Runtime>::certify_smith( + RuntimeOrigin::signed(3), + 5 + )); + // Should not be promoted again + assert!(!System::events().iter().any(|record| record.event + == RuntimeEvent::Smith(Event::<Runtime>::SmithMembershipAdded { idty_index: 5 },))); + }); +} + #[test] fn should_have_checks_on_certify() { new_test_ext(GenesisConfig { @@ -510,6 +556,80 @@ fn certifying_on_different_status() { }); } +#[test] +fn certifying_an_online_smith() { + new_test_ext(GenesisConfig { + initial_smiths: btreemap![ + 1 => (false, vec![2, 3, 4]), + 2 => (false, vec![3, 4]), + 3 => (false, vec![1, 2]), + 4 => (false, vec![]), + ], + }) + .execute_with(|| { + // Go online to be able to invite+certify + Pallet::<Runtime>::on_smith_goes_online(1); + Pallet::<Runtime>::on_smith_goes_online(2); + Pallet::<Runtime>::on_smith_goes_online(3); + assert_ok!(Pallet::<Runtime>::invite_smith(RuntimeOrigin::signed(1), 5)); + assert_ok!(Pallet::<Runtime>::accept_invitation(RuntimeOrigin::signed( + 5 + ))); + Pallet::<Runtime>::on_new_session(2); + assert_ok!(Pallet::<Runtime>::certify_smith( + RuntimeOrigin::signed(1), + 5 + )); + Pallet::<Runtime>::on_new_session(3); + assert_ok!(Pallet::<Runtime>::certify_smith( + RuntimeOrigin::signed(2), + 5 + )); + // Smith can expire + assert_eq!( + Smiths::<Runtime>::get(5), + Some(SmithMeta { + status: Smith, + expires_on: Some(8), + issued_certs: vec![], + received_certs: vec![1, 2] + }) + ); + assert_eq!(ExpiresOn::<Runtime>::get(7), Some(vec![5])); + assert_eq!(ExpiresOn::<Runtime>::get(8), Some(vec![5])); + + Pallet::<Runtime>::on_smith_goes_online(5); + // After going online, the expiration disappears + assert_eq!( + Smiths::<Runtime>::get(5), + Some(SmithMeta { + status: Smith, + expires_on: None, + issued_certs: vec![], + received_certs: vec![1, 2] + }) + ); + // ExpiresOn is not unscheduled, but as expires_on has switched to None it's not a problem + assert_eq!(ExpiresOn::<Runtime>::get(7), Some(vec![5])); + assert_eq!(ExpiresOn::<Runtime>::get(8), Some(vec![5])); + + // We can receive certification without postponing the expiration (because we are online) + assert_ok!(Pallet::<Runtime>::certify_smith( + RuntimeOrigin::signed(3), + 5 + )); + assert_eq!( + Smiths::<Runtime>::get(5), + Some(SmithMeta { + status: Smith, + expires_on: None, + issued_certs: vec![], + received_certs: vec![1, 2, 3] + }) + ); + }); +} + #[test] fn invitation_on_non_wot_member() { new_test_ext(GenesisConfig {