From 6100fe2c8a1d36ac16321a09ef5922e8b0701aba Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Mon, 29 May 2023 21:46:58 +0200
Subject: [PATCH] fix(!115): remove IdtyEvent type

---
 pallets/duniter-wot/src/lib.rs          | 26 +++++++++-----
 pallets/duniter-wot/src/tests.rs        | 16 +++++++--
 pallets/identity/src/lib.rs             | 46 +++++++++++++++++++------
 pallets/identity/src/tests.rs           |  8 ++++-
 pallets/identity/src/traits.rs          |  4 +--
 pallets/identity/src/types.rs           | 14 --------
 runtime/common/src/handlers.rs          | 19 ++++++----
 runtime/gdev/tests/integration_tests.rs | 21 ++++++++---
 8 files changed, 103 insertions(+), 51 deletions(-)

diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index 50a41c70b..ee76d7b04 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -32,7 +32,7 @@ use frame_support::dispatch::UnfilteredDispatchable;
 use frame_support::pallet_prelude::*;
 use frame_system::RawOrigin;
 use pallet_certification::traits::SetNextIssuableOn;
-use pallet_identity::{IdtyEvent, IdtyStatus};
+use pallet_identity::IdtyStatus;
 use sp_runtime::traits::IsMember;
 
 type IdtyIndex = u32;
@@ -300,27 +300,33 @@ where
 
 // implement identity event handler
 impl<T: Config<I>, I: 'static> pallet_identity::traits::OnIdtyChange<T> for Pallet<T, I> {
-    fn on_idty_change(idty_index: IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight {
+    fn on_idty_change(_idty_index: IdtyIndex, idty_event: &pallet_identity::Event<T>) -> Weight {
         match idty_event {
-            IdtyEvent::Created { creator } => {
+            pallet_identity::Event::IdtyCreated {
+                creator,
+                owner_key: _owner_key,
+                idty_index,
+            } => {
                 if let Err(e) = <pallet_certification::Pallet<T, I>>::do_add_cert_checked(
-                    *creator, idty_index, true,
+                    *creator,
+                    *idty_index,
+                    true,
                 ) {
                     sp_std::if_std! {
                         println!("fail to force add cert: {:?}", e)
                     }
                 }
             }
-            IdtyEvent::Validated => {
+            pallet_identity::Event::IdtyValidated { idty_index } => {
                 // auto claim membership on main wot
-                <pallet_membership::Pallet<T, I>>::try_claim_membership(idty_index);
+                <pallet_membership::Pallet<T, I>>::try_claim_membership(*idty_index);
             }
-            IdtyEvent::Removed { status } => {
+            pallet_identity::Event::IdtyRemoved { idty_index, status } => {
                 if *status != IdtyStatus::Validated {
                     if let Err(e) =
                         <pallet_certification::Pallet<T, I>>::remove_all_certs_received_by(
                             frame_system::Origin::<T>::Root.into(),
-                            idty_index,
+                            *idty_index,
                         )
                     {
                         sp_std::if_std! {
@@ -329,7 +335,9 @@ impl<T: Config<I>, I: 'static> pallet_identity::traits::OnIdtyChange<T> for Pall
                     }
                 }
             }
-            IdtyEvent::Confirmed | IdtyEvent::ChangedOwnerKey { .. } => {}
+            pallet_identity::Event::IdtyConfirmed { .. }
+            | pallet_identity::Event::IdtyChangedOwnerKey { .. } => {}
+            _ => {} // TODO: pourquoi est-ce nécessaire ?
         }
         Weight::zero()
     }
diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs
index 7f98d970f..034c75c39 100644
--- a/pallets/duniter-wot/src/tests.rs
+++ b/pallets/duniter-wot/src/tests.rs
@@ -159,6 +159,7 @@ fn test_create_idty_ok() {
         // 2 events should have occurred: IdtyCreated and NewCert
         System::assert_has_event(RuntimeEvent::Identity(
             pallet_identity::Event::IdtyCreated {
+                creator: 1,
                 idty_index: 6,
                 owner_key: 6,
             },
@@ -305,7 +306,10 @@ fn test_revoke_idty() {
         ));
 
         System::assert_has_event(RuntimeEvent::Identity(
-            pallet_identity::Event::IdtyRemoved { idty_index: 2 },
+            pallet_identity::Event::IdtyRemoved {
+                idty_index: 2,
+                status: IdtyStatus::Validated,
+            },
         ));
     });
 }
@@ -330,7 +334,10 @@ fn test_idty_membership_expire() {
         ));
         // membership expiry should not trigger identity removal
         assert!(!System::events().iter().any(|record| record.event
-            == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved { idty_index: 3 })));
+            == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved {
+                idty_index: 3,
+                status: IdtyStatus::Validated
+            })));
         // it should be moved to pending membership instead
         assert!(Membership::pending_membership(3).is_some());
 
@@ -340,7 +347,10 @@ fn test_idty_membership_expire() {
             pallet_membership::Event::PendingMembershipExpired(3),
         ));
         System::assert_has_event(RuntimeEvent::Identity(
-            pallet_identity::Event::IdtyRemoved { idty_index: 3 },
+            pallet_identity::Event::IdtyRemoved {
+                idty_index: 3,
+                status: IdtyStatus::Validated,
+            },
         ));
 
         // Charlie's identity should be removed at block #11
diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs
index 06514db71..4a7396372 100644
--- a/pallets/identity/src/lib.rs
+++ b/pallets/identity/src/lib.rs
@@ -228,6 +228,7 @@ pub mod pallet {
         /// A new identity has been created
         /// [idty_index, owner_key]
         IdtyCreated {
+            creator: T::IdtyIndex,
             idty_index: T::IdtyIndex,
             owner_key: T::AccountId,
         },
@@ -247,7 +248,10 @@ pub mod pallet {
         },
         /// An identity has been removed
         /// [idty_index]
-        IdtyRemoved { idty_index: T::IdtyIndex },
+        IdtyRemoved {
+            idty_index: T::IdtyIndex,
+            status: IdtyStatus,
+        },
     }
 
     // CALLS //
@@ -314,10 +318,18 @@ pub mod pallet {
             IdentitiesRemovableOn::<T>::append(removable_on, (idty_index, IdtyStatus::Created));
             IdentityIndexOf::<T>::insert(owner_key.clone(), idty_index);
             Self::deposit_event(Event::IdtyCreated {
+                creator: creator.clone(),
                 idty_index,
-                owner_key,
+                owner_key: owner_key.clone(),
             });
-            T::OnIdtyChange::on_idty_change(idty_index, &IdtyEvent::Created { creator });
+            T::OnIdtyChange::on_idty_change(
+                idty_index,
+                &Event::IdtyCreated {
+                    creator,
+                    idty_index,
+                    owner_key,
+                },
+            );
             Ok(().into())
         }
 
@@ -358,10 +370,17 @@ pub mod pallet {
             <IdentitiesNames<T>>::insert(idty_name.clone(), ());
             Self::deposit_event(Event::IdtyConfirmed {
                 idty_index,
-                owner_key: who,
-                name: idty_name,
+                owner_key: who.clone(),
+                name: idty_name.clone(),
             });
-            T::OnIdtyChange::on_idty_change(idty_index, &IdtyEvent::Confirmed);
+            T::OnIdtyChange::on_idty_change(
+                idty_index,
+                &Event::IdtyConfirmed {
+                    idty_index,
+                    owner_key: who,
+                    name: idty_name,
+                },
+            );
             Ok(().into())
         }
 
@@ -392,7 +411,7 @@ pub mod pallet {
 
             <Identities<T>>::insert(idty_index, idty_value);
             Self::deposit_event(Event::IdtyValidated { idty_index });
-            T::OnIdtyChange::on_idty_change(idty_index, &IdtyEvent::Validated);
+            T::OnIdtyChange::on_idty_change(idty_index, &Event::IdtyValidated { idty_index });
 
             Ok(().into())
         }
@@ -471,8 +490,9 @@ pub mod pallet {
             });
             T::OnIdtyChange::on_idty_change(
                 idty_index,
-                &IdtyEvent::ChangedOwnerKey {
-                    new_owner_key: new_key,
+                &Event::IdtyChangedOwnerKey {
+                    idty_index,
+                    new_owner_key: new_key.clone(),
                 },
             );
 
@@ -671,10 +691,14 @@ pub mod pallet {
                 if let Some((old_owner_key, _last_change)) = idty_val.old_owner_key {
                     frame_system::Pallet::<T>::dec_sufficients(&old_owner_key);
                 }
-                Self::deposit_event(Event::IdtyRemoved { idty_index });
+                Self::deposit_event(Event::IdtyRemoved {
+                    idty_index,
+                    status: idty_val.status,
+                });
                 T::OnIdtyChange::on_idty_change(
                     idty_index,
-                    &IdtyEvent::Removed {
+                    &Event::IdtyRemoved {
+                        idty_index,
                         status: idty_val.status,
                     },
                 );
diff --git a/pallets/identity/src/tests.rs b/pallets/identity/src/tests.rs
index f04d9d7cb..583363f06 100644
--- a/pallets/identity/src/tests.rs
+++ b/pallets/identity/src/tests.rs
@@ -16,7 +16,7 @@
 
 use crate::mock::*;
 use crate::{
-    Error, GenesisIdty, IdtyName, IdtyValue, NewOwnerKeyPayload, RevocationPayload,
+    Error, GenesisIdty, IdtyName, IdtyStatus, IdtyValue, NewOwnerKeyPayload, RevocationPayload,
     NEW_OWNER_KEY_PAYLOAD_PREFIX, REVOCATION_PAYLOAD_PREFIX,
 };
 use codec::Encode;
@@ -103,6 +103,7 @@ fn test_create_identity_ok() {
         ));
 
         System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated {
+            creator: 1,
             idty_index: 2,
             owner_key: account(2).id,
         }));
@@ -129,6 +130,7 @@ fn test_create_identity_but_not_confirm_it() {
 
         System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyRemoved {
             idty_index: 2,
+            status: IdtyStatus::Created,
         }));
 
         // We shoud be able to recreate the identity
@@ -139,6 +141,7 @@ fn test_create_identity_but_not_confirm_it() {
         ));
 
         System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated {
+            creator: 1,
             idty_index: 3,
             owner_key: account(2).id,
         }));
@@ -161,6 +164,7 @@ fn test_idty_creation_period() {
         ));
 
         System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated {
+            creator: 1,
             idty_index: 2,
             owner_key: account(2).id,
         }));
@@ -182,6 +186,7 @@ fn test_idty_creation_period() {
         ));
 
         System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated {
+            creator: 1,
             idty_index: 3,
             owner_key: account(3).id,
         }));
@@ -496,6 +501,7 @@ fn test_idty_revocation() {
         }));
         System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyRemoved {
             idty_index: 1,
+            status: IdtyStatus::Validated,
         }));
 
         run_to_block(2);
diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs
index 54b50a06c..274961841 100644
--- a/pallets/identity/src/traits.rs
+++ b/pallets/identity/src/traits.rs
@@ -55,13 +55,13 @@ pub trait IdtyNameValidator {
 }
 
 pub trait OnIdtyChange<T: Config> {
-    fn on_idty_change(idty_index: T::IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight;
+    fn on_idty_change(idty_index: T::IdtyIndex, idty_event: &Event<T>) -> Weight;
 }
 
 #[impl_for_tuples(5)]
 #[allow(clippy::let_and_return)]
 impl<T: Config> OnIdtyChange<T> for Tuple {
-    fn on_idty_change(idty_index: T::IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight {
+    fn on_idty_change(idty_index: T::IdtyIndex, idty_event: &Event<T>) -> Weight {
         let mut weight = Weight::zero();
         for_tuples!( #( weight = weight.saturating_add(Tuple::on_idty_change(idty_index, idty_event)); )* );
         weight
diff --git a/pallets/identity/src/types.rs b/pallets/identity/src/types.rs
index 1d633e293..f7d77306e 100644
--- a/pallets/identity/src/types.rs
+++ b/pallets/identity/src/types.rs
@@ -23,20 +23,6 @@ use scale_info::TypeInfo;
 use serde::{Deserialize, Serialize};
 use sp_std::vec::Vec;
 
-/// events related to identity
-pub enum IdtyEvent<T: crate::Config> {
-    /// creation of a new identity by an other
-    Created { creator: T::IdtyIndex },
-    /// confirmation of an identity (with a given name)
-    Confirmed,
-    /// validation of an identity
-    Validated,
-    /// changing the owner key of the identity
-    ChangedOwnerKey { new_owner_key: T::AccountId },
-    /// removing an identity
-    Removed { status: IdtyStatus },
-}
-
 /// name of the identity, ascii encoded
 #[derive(Encode, Decode, Default, Clone, PartialEq, Eq, PartialOrd, Ord, RuntimeDebug)]
 pub struct IdtyName(pub Vec<u8>);
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index 95a6ccabd..7b33941a5 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -21,7 +21,6 @@ use frame_support::instances::{Instance1, Instance2};
 use frame_support::pallet_prelude::Weight;
 use frame_support::traits::Get;
 use frame_support::Parameter;
-use pallet_identity::IdtyEvent;
 use sp_runtime::traits::IsMember;
 
 // new session handler
@@ -45,16 +44,19 @@ where
     T: pallet_identity::Config<IdtyIndex = IdtyIndex, IdtyData = IdtyData>,
     T: pallet_universal_dividend::Config,
 {
-    fn on_idty_change(idty_index: IdtyIndex, idty_event: &IdtyEvent<T>) -> Weight {
+    fn on_idty_change(_idty_index: IdtyIndex, idty_event: &pallet_identity::Event<T>) -> Weight {
         match idty_event {
-            IdtyEvent::Validated => {
+            pallet_identity::Event::IdtyValidated { idty_index: _ } => {
                 // when identity is validated, it starts getting right to UD
                 // but this is handeled by membership event handler (MembershipAcquired)
             }
-            IdtyEvent::ChangedOwnerKey { new_owner_key } => {
+            pallet_identity::Event::IdtyChangedOwnerKey {
+                idty_index,
+                new_owner_key,
+            } => {
                 if let Err(e) = pallet_authority_members::Pallet::<T>::change_owner_key(
-                    idty_index,
-                    new_owner_key.clone(),
+                    *idty_index,
+                    (*new_owner_key).clone(),
                 ) {
                     log::error!(
                         "on_idty_change: pallet_authority_members.change_owner_key(): {:?}",
@@ -62,7 +64,10 @@ where
                     );
                 }
             }
-            IdtyEvent::Created { .. } | IdtyEvent::Confirmed | IdtyEvent::Removed { .. } => {}
+            pallet_identity::Event::IdtyCreated { .. }
+            | pallet_identity::Event::IdtyConfirmed { .. }
+            | pallet_identity::Event::IdtyRemoved { .. } => {}
+            &_ => {} // TODO: why?
         }
         Weight::zero()
     }
diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index 22d1fb056..3c6bab98b 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -21,6 +21,7 @@ use frame_support::traits::{Get, PalletInfo, StorageInfo, StorageInfoTrait};
 use frame_support::{assert_noop, assert_ok};
 use frame_support::{StorageHasher, Twox128};
 use gdev_runtime::*;
+use pallet_identity::IdtyStatus;
 use sp_keyring::AccountKeyring;
 use sp_runtime::MultiAddress;
 
@@ -100,7 +101,10 @@ fn test_remove_identity() {
             account: AccountKeyring::Dave.to_account_id(),
         }));
         System::assert_has_event(RuntimeEvent::Identity(
-            pallet_identity::Event::IdtyRemoved { idty_index: 4 },
+            pallet_identity::Event::IdtyRemoved {
+                idty_index: 4,
+                status: IdtyStatus::Validated,
+            },
         ));
     });
 }
@@ -167,7 +171,10 @@ fn test_membership_expiry() {
         ));
         // membership expiry should not trigger identity removal
         assert!(!System::events().iter().any(|record| record.event
-            == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved { idty_index: 1 })));
+            == RuntimeEvent::Identity(pallet_identity::Event::IdtyRemoved {
+                idty_index: 1,
+                status: IdtyStatus::Validated
+            })));
     });
 }
 
@@ -234,7 +241,10 @@ fn test_remove_identity_after_one_ud() {
             },
         ));
         System::assert_has_event(RuntimeEvent::Identity(
-            pallet_identity::Event::IdtyRemoved { idty_index: 4 },
+            pallet_identity::Event::IdtyRemoved {
+                idty_index: 4,
+                status: IdtyStatus::Validated,
+            },
         ));
 
         // Verify state
@@ -356,7 +366,10 @@ fn test_remove_smith_identity() {
             pallet_membership::Event::MembershipRevoked(3),
         ));
         System::assert_has_event(RuntimeEvent::Identity(
-            pallet_identity::Event::IdtyRemoved { idty_index: 3 },
+            pallet_identity::Event::IdtyRemoved {
+                idty_index: 3,
+                status: IdtyStatus::Validated,
+            },
         ));
     });
 }
-- 
GitLab