From f4da450bbd4baeea440df5434144a4cf8856c351 Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Wed, 27 Dec 2023 07:54:43 +0100
Subject: [PATCH] test(smith-members): change_owner_key
---
pallets/authority-members/src/lib.rs | 23 +++---
pallets/duniter-wot/src/lib.rs | 13 ----
pallets/identity/src/lib.rs | 2 -
pallets/identity/src/traits.rs | 5 --
pallets/smith-members/src/lib.rs | 8 +--
runtime/common/src/pallets_config.rs | 6 +-
runtime/gdev/tests/integration_tests.rs | 95 +++++++++++++++++++++----
7 files changed, 95 insertions(+), 57 deletions(-)
diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs
index a3b03d0d0..f57ff7d39 100644
--- a/pallets/authority-members/src/lib.rs
+++ b/pallets/authority-members/src/lib.rs
@@ -530,27 +530,22 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> {
let members_ids_to_add = IncomingAuthorities::<T>::take();
let members_ids_to_del = OutgoingAuthorities::<T>::take();
- // TODO: what if we have outgoing members but no incoming ones?
- if members_ids_to_add.is_empty() {
- if members_ids_to_del.is_empty() {
- // when no change to the set of autorities, return None
- return None;
- } else {
- Self::deposit_event(Event::OutgoingAuthorities {
- members: members_ids_to_del.clone(),
- });
- }
- } else {
- Self::deposit_event(Event::IncomingAuthorities {
- members: members_ids_to_add.clone(),
- });
+ if members_ids_to_add.is_empty() && members_ids_to_del.is_empty() {
+ // when no change to the set of autorities, return None
+ return None;
}
for member_id in members_ids_to_add.iter() {
T::OnIncomingMember::on_incoming_member(*member_id);
+ Self::deposit_event(Event::IncomingAuthorities {
+ members: members_ids_to_add.clone(),
+ });
}
for member_id in members_ids_to_del.iter() {
T::OnOutgoingMember::on_outgoing_member(*member_id);
+ Self::deposit_event(Event::OutgoingAuthorities {
+ members: members_ids_to_del.clone(),
+ });
}
// updates the list of OnlineAuthorities and returns the list of their key
diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index 39d8d684c..7cf49c3a7 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -158,19 +158,6 @@ where
// smith subwot can never prevent from creating identity
Ok(())
}
-
- // identity change owner key cheks
- fn change_owner_key(idty_index: IdtyIndex) -> Result<(), DispatchError> {
- // sub WoT prevents from changing identity
- if T::IsSubWot::get() {
- ensure!(
- !pallet_membership::Pallet::<T, I>::is_member(&idty_index),
- Error::<T, I>::NotAllowedToChangeIdtyAddress
- );
- }
- // no constraints for main wot
- Ok(())
- }
}
// implement cert call checks
diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs
index c89d77aac..05ee00f7d 100644
--- a/pallets/identity/src/lib.rs
+++ b/pallets/identity/src/lib.rs
@@ -413,8 +413,6 @@ pub mod pallet {
Error::<T>::OwnerKeyAlreadyUsed
);
- T::CheckIdtyCallAllowed::change_owner_key(idty_index)?;
-
let block_number = frame_system::Pallet::<T>::block_number();
let maybe_old_old_owner_key =
if let Some((old_owner_key, last_change)) = idty_value.old_owner_key {
diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs
index ffc230aa7..9b741545e 100644
--- a/pallets/identity/src/traits.rs
+++ b/pallets/identity/src/traits.rs
@@ -20,7 +20,6 @@ use impl_trait_for_tuples::impl_for_tuples;
pub trait CheckIdtyCallAllowed<T: Config> {
fn check_create_identity(creator: T::IdtyIndex) -> Result<(), DispatchError>;
- fn change_owner_key(idty_index: T::IdtyIndex) -> Result<(), DispatchError>;
}
#[impl_for_tuples(5)]
@@ -29,10 +28,6 @@ impl<T: Config> CheckIdtyCallAllowed<T> for Tuple {
for_tuples!( #( Tuple::check_create_identity(creator)?; )* );
Ok(())
}
- fn change_owner_key(idty_index: T::IdtyIndex) -> Result<(), DispatchError> {
- for_tuples!( #( Tuple::change_owner_key(idty_index)?; )* );
- Ok(())
- }
}
pub trait IdtyNameValidator {
diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 5f8120461..66ab569c5 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -516,7 +516,7 @@ impl<T: Config> Pallet<T> {
}
// TODO: return what?
- fn smith_goes_offline(idty_index: T::IdtyIndex) {
+ pub fn smith_goes_offline(idty_index: T::IdtyIndex) {
if let Some(smith_meta) = Smiths::<T>::get(idty_index) {
if smith_meta.expires_on.is_none() {
Smiths::<T>::mutate(idty_index, |maybe_smith_meta| {
@@ -544,11 +544,7 @@ impl<T: Config> Pallet<T> {
let Some(smith) = Smiths::<T>::get(idty_id) else {
return false;
};
- let mut count = 0u32;
- for _ in smith.received_certs {
- count += 1;
- }
- count >= T::MinCertForMembership::get()
+ smith.status == SmithStatus::Smith
}
}
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index 90a9b2202..1c64d6bb8 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/runtime/common/src/pallets_config.rs
@@ -241,9 +241,9 @@ macro_rules! pallets_config {
type MaxAuthorities = MaxAuthorities;
type RemoveMemberOrigin = EnsureRoot<Self::AccountId>;
type WeightInfo = common_runtime::weights::pallet_authority_members::WeightInfo<Runtime>;
- type OnBlacklistedMember = ();
- type OnIncomingMember = ();
- type OnOutgoingMember = ();
+ type OnBlacklistedMember = SmithMembers;
+ type OnIncomingMember = SmithMembers;
+ type OnOutgoingMember = SmithMembers;
}
impl pallet_authorship::Config for Runtime {
type EventHandler = ImOnline;
diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index 99de08be0..62d0c5288 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -24,7 +24,7 @@ use frame_support::{assert_err, assert_noop, assert_ok};
use frame_support::{StorageHasher, Twox128};
use gdev_runtime::*;
use pallet_membership::MembershipRemovalReason;
-use pallet_smith_members::SmithStatus;
+use pallet_smith_members::{SmithMeta, SmithStatus};
use sp_core::Encode;
use sp_keyring::AccountKeyring;
use sp_runtime::MultiAddress;
@@ -690,6 +690,15 @@ fn test_smith_certification() {
});
}
+fn create_dummy_session_keys() -> gdev_runtime::opaque::SessionKeys {
+ gdev_runtime::opaque::SessionKeys {
+ grandpa: sp_core::ed25519::Public([0u8; 32]).into(),
+ babe: sp_core::sr25519::Public([0u8; 32]).into(),
+ im_online: sp_core::sr25519::Public([0u8; 32]).into(),
+ authority_discovery: sp_core::sr25519::Public([0u8; 32]).into(),
+ }
+}
+
/// test the full process to join smith from main wot member to authority member
#[test]
fn test_smith_process() {
@@ -703,12 +712,7 @@ fn test_smith_process() {
let bob = AccountKeyring::Bob.to_account_id();
let charlie = AccountKeyring::Charlie.to_account_id();
let dave = AccountKeyring::Dave.to_account_id();
- let dummy_session_keys = gdev_runtime::opaque::SessionKeys {
- grandpa: sp_core::ed25519::Public([0u8; 32]).into(),
- babe: sp_core::sr25519::Public([0u8; 32]).into(),
- im_online: sp_core::sr25519::Public([0u8; 32]).into(),
- authority_discovery: sp_core::sr25519::Public([0u8; 32]).into(),
- };
+ let dummy_session_keys = create_dummy_session_keys();
// Eve can not request smith membership because not member of the smith wot
// no more membership request
@@ -1241,14 +1245,26 @@ fn test_link_account() {
fn test_change_owner_key() {
ExtBuilder::new(1, 3, 4).build().execute_with(|| {
let genesis_hash = System::block_hash(0);
- let dave = AccountKeyring::Dave.to_account_id();
+ let charlie = AccountKeyring::Charlie.to_account_id();
let ferdie = AccountKeyring::Ferdie.to_account_id();
- let payload = (b"icok", genesis_hash, 4u32, dave.clone()).encode();
+ let payload = (b"icok", genesis_hash, 3u32, charlie.clone()).encode();
let signature = AccountKeyring::Ferdie.sign(&payload);
+ SmithMembers::smith_goes_offline(3);
+ // Charlie is now offline smith
assert_eq!(
- frame_system::Pallet::<Runtime>::get(&dave).linked_idty,
- Some(4)
+ SmithMembers::smiths(3),
+ Some(SmithMeta {
+ status: SmithStatus::Smith,
+ expires_on: Some(48),
+ issued_certs: vec![1, 2],
+ received_certs: vec![1, 2]
+ })
+ );
+
+ assert_eq!(
+ frame_system::Pallet::<Runtime>::get(&charlie).linked_idty,
+ Some(3)
);
assert_eq!(
frame_system::Pallet::<Runtime>::get(&ferdie).linked_idty,
@@ -1256,13 +1272,65 @@ fn test_change_owner_key() {
);
// Dave can change his owner key to Ferdie's
assert_ok!(Identity::change_owner_key(
- frame_system::RawOrigin::Signed(dave.clone()).into(),
+ frame_system::RawOrigin::Signed(charlie.clone()).into(),
ferdie.clone(),
signature.into()
));
assert_eq!(
frame_system::Pallet::<Runtime>::get(&ferdie).linked_idty,
- Some(4)
+ Some(3)
+ );
+
+ // Charlie is still an offline smith
+ assert_eq!(
+ SmithMembers::smiths(3),
+ Some(SmithMeta {
+ status: SmithStatus::Smith,
+ expires_on: Some(48),
+ issued_certs: vec![1, 2],
+ received_certs: vec![1, 2]
+ })
+ );
+
+ // Ferdie can set its session_keys and go online
+ frame_system::Pallet::<Runtime>::inc_providers(&ferdie);
+ assert_ok!(AuthorityMembers::set_session_keys(
+ frame_system::RawOrigin::Signed(AccountKeyring::Ferdie.to_account_id()).into(),
+ create_dummy_session_keys()
+ ));
+ assert_ok!(AuthorityMembers::go_online(
+ frame_system::RawOrigin::Signed(AccountKeyring::Ferdie.to_account_id()).into()
+ ));
+
+ // Charlie is still an offline smith
+ assert_eq!(
+ SmithMembers::smiths(3),
+ Some(SmithMeta {
+ status: SmithStatus::Smith,
+ expires_on: Some(48),
+ issued_certs: vec![1, 2],
+ received_certs: vec![1, 2]
+ })
+ );
+
+ run_to_block(25);
+
+ System::assert_has_event(RuntimeEvent::AuthorityMembers(
+ pallet_authority_members::Event::IncomingAuthorities { members: vec![3] },
+ ));
+ System::assert_has_event(RuntimeEvent::AuthorityMembers(
+ pallet_authority_members::Event::OutgoingAuthorities { members: vec![1] },
+ ));
+
+ // "Charlie" (idty 3) is now online because its identity is mapped to Ferdies's key
+ assert_eq!(
+ SmithMembers::smiths(3),
+ Some(SmithMeta {
+ status: SmithStatus::Smith,
+ expires_on: None,
+ issued_certs: vec![1, 2],
+ received_certs: vec![1, 2]
+ })
);
})
}
@@ -1378,7 +1446,6 @@ fn test_killed_account() {
})
}
-// TODO: test_smith_member_cant_change_its_idty_address
// TODO: test_smith_member_can_revoke_its_idty
// TODO: test_revoke_idty
// TODO: test_non_smith_can_not_issue_smith_cert
--
GitLab