From e69b70e8eddfb43bcefb3318e1fe5248a570c1c6 Mon Sep 17 00:00:00 2001
From: librelois <c@elo.tf>
Date: Fri, 21 Jan 2022 23:16:09 +0100
Subject: [PATCH] =?UTF-8?q?feat(membership):=C2=A0add=20optional=20metadat?=
 =?UTF-8?q?a=20in=20pending=20membership?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 pallets/duniter-wot/src/lib.rs       | 13 ++++++++----
 pallets/duniter-wot/src/tests.rs     |  2 +-
 pallets/duniter-wot/src/types.rs     | 12 +++++------
 pallets/membership/src/lib.rs        | 25 ++++++++++-------------
 pallets/membership/src/tests.rs      | 30 ++++++++++++++++++----------
 runtime/common/src/entities.rs       |  7 +++++++
 runtime/common/src/handlers.rs       | 15 ++++++++------
 runtime/common/src/pallets_config.rs |  2 +-
 runtime/g1/src/lib.rs                |  4 ++--
 runtime/gdev/src/lib.rs              |  4 ++--
 runtime/gtest/src/lib.rs             |  4 ++--
 11 files changed, 67 insertions(+), 51 deletions(-)

diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs
index 0c5e8087c..8088bae91 100644
--- a/pallets/duniter-wot/src/lib.rs
+++ b/pallets/duniter-wot/src/lib.rs
@@ -36,6 +36,7 @@ use frame_support::pallet_prelude::*;
 use frame_system::RawOrigin;
 use pallet_certification::traits::SetNextIssuableOn;
 use pallet_identity::{IdtyEvent, IdtyStatus};
+use sp_membership::traits::IsInPendingMemberships;
 use sp_runtime::traits::IsMember;
 
 type IdtyIndex = u32;
@@ -107,11 +108,15 @@ where
         }
     }
     fn can_confirm_identity(idty_index: IdtyIndex) -> bool {
-        pallet_membership::Pallet::<T, I>::request_membership(RawOrigin::Root.into(), idty_index)
-            .is_ok()
+        pallet_membership::Pallet::<T, I>::request_membership(
+            RawOrigin::Root.into(),
+            idty_index,
+            (),
+        )
+        .is_ok()
     }
     fn can_validate_identity(idty_index: IdtyIndex) -> bool {
-        pallet_membership::Pallet::<T, I>::claim_membership(RawOrigin::Root.into(), idty_index, ())
+        pallet_membership::Pallet::<T, I>::claim_membership(RawOrigin::Root.into(), idty_index)
             .is_ok()
     }
 }
@@ -261,7 +266,7 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::OnNewcert<IdtyIndex
             if receiver_received_count == T::MinReceivedCertToBeAbleToIssueCert::get() {
                 Self::do_apply_first_issuable_on(receiver);
             }
-        } else if pallet_membership::Pallet::<T, I>::pending_membership(receiver).is_some()
+        } else if pallet_membership::Pallet::<T, I>::is_in_pending_memberships(receiver)
             && receiver_received_count >= T::MinCertForMembership::get()
         {
             // TODO insert `receiver` in distance queue
diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs
index d87f3bb9f..a688a06d7 100644
--- a/pallets/duniter-wot/src/tests.rs
+++ b/pallets/duniter-wot/src/tests.rs
@@ -237,7 +237,7 @@ fn test_idty_membership_expire_them_requested() {
 
         // Charlie should be able to request membership
         run_to_block(6);
-        assert_ok!(Membership::request_membership(Origin::signed(3), 3));
+        assert_ok!(Membership::request_membership(Origin::signed(3), 3, ()));
 
         // Charlie should re-enter in the wot immediatly
         let events = System::events();
diff --git a/pallets/duniter-wot/src/types.rs b/pallets/duniter-wot/src/types.rs
index ffac045f8..b6b0675ae 100644
--- a/pallets/duniter-wot/src/types.rs
+++ b/pallets/duniter-wot/src/types.rs
@@ -14,10 +14,10 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with Substrate-Libre-Currency. If not, see <https://www.gnu.org/licenses/>.
 
-use pallet_identity::IdtyStatus;
-
 use crate::{Config, IdtyIndex};
 use frame_support::pallet_prelude::*;
+use pallet_identity::IdtyStatus;
+use sp_membership::traits::IsInPendingMemberships;
 use sp_runtime::traits::IsMember;
 
 pub struct AddCertOrigin<T, I>(core::marker::PhantomData<(T, I)>);
@@ -39,8 +39,7 @@ impl<T: Config<I>, I: 'static> EnsureOrigin<(T::Origin, IdtyIndex, IdtyIndex)>
                                 IdtyStatus::ConfirmedByOwner => Ok(()),
                                 IdtyStatus::Created => Err(o),
                                 IdtyStatus::Disabled => {
-                                    if pallet_membership::Pallet::<T, I>::pending_membership(&o.2)
-                                        .is_some()
+                                    if pallet_membership::Pallet::<T, I>::is_in_pending_memberships(o.2)
                                     {
                                         Ok(())
                                     } else {
@@ -49,10 +48,9 @@ impl<T: Config<I>, I: 'static> EnsureOrigin<(T::Origin, IdtyIndex, IdtyIndex)>
                                 }
                                 IdtyStatus::Validated => {
                                     if pallet_membership::Pallet::<T, I>::is_member(&o.2)
-                                        || pallet_membership::Pallet::<T, I>::pending_membership(
-                                            &o.2,
+                                        || pallet_membership::Pallet::<T, I>::is_in_pending_memberships(
+                                            o.2,
                                         )
-                                        .is_some()
                                     {
                                         Ok(())
                                     } else {
diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs
index 14d5f189f..37470047f 100644
--- a/pallets/membership/src/lib.rs
+++ b/pallets/membership/src/lib.rs
@@ -135,7 +135,7 @@ pub mod pallet {
     #[pallet::storage]
     #[pallet::getter(fn pending_membership)]
     pub type PendingMembership<T: Config<I>, I: 'static = ()> =
-        StorageMap<_, Blake2_128Concat, T::IdtyId, T::BlockNumber, OptionQuery>;
+        StorageMap<_, Blake2_128Concat, T::IdtyId, T::MetaData, OptionQuery>;
 
     #[pallet::storage]
     #[pallet::getter(fn pending_memberships_expire_on)]
@@ -226,6 +226,7 @@ pub mod pallet {
         pub fn request_membership(
             origin: OriginFor<T>,
             idty_id: T::IdtyId,
+            metadata: T::MetaData,
         ) -> DispatchResultWithPostInfo {
             let allowed =
                 match T::IsOriginAllowedToUseIdty::is_origin_allowed_to_use_idty(&origin, &idty_id)
@@ -256,7 +257,7 @@ pub mod pallet {
             let block_number = frame_system::pallet::Pallet::<T>::block_number();
             let expire_on = block_number + T::PendingMembershipPeriod::get();
 
-            PendingMembership::<T, I>::insert(idty_id, expire_on);
+            PendingMembership::<T, I>::insert(idty_id, metadata);
             PendingMembershipsExpireOn::<T, I>::append(expire_on, idty_id);
             Self::deposit_event(Event::MembershipRequested(idty_id));
             T::OnEvent::on_event(&sp_membership::Event::MembershipRequested(idty_id));
@@ -268,8 +269,8 @@ pub mod pallet {
         pub fn claim_membership(
             origin: OriginFor<T>,
             idty_id: T::IdtyId,
-            metadata: T::MetaData,
         ) -> DispatchResultWithPostInfo {
+            // Verify phase
             if Membership::<T, I>::contains_key(&idty_id) {
                 return Err(Error::<T, I>::MembershipAlreadyAcquired.into());
             }
@@ -290,11 +291,13 @@ pub mod pallet {
                 return Err(Error::<T, I>::IdtyNotAllowedToClaimMembership.into());
             }
 
-            if !PendingMembership::<T, I>::contains_key(&idty_id) {
-                return Err(Error::<T, I>::MembershipRequestNotFound.into());
-            }
+            let metadata = PendingMembership::<T, I>::take(&idty_id)
+                .ok_or(Error::<T, I>::MembershipRequestNotFound)?;
 
-            let _ = Self::do_claim_membership(idty_id, metadata);
+            // Apply phase
+            Self::do_renew_membership_inner(idty_id);
+            Self::deposit_event(Event::MembershipAcquired(idty_id));
+            T::OnEvent::on_event(&sp_membership::Event::MembershipAcquired(idty_id, metadata));
 
             Ok(().into())
         }
@@ -355,14 +358,6 @@ pub mod pallet {
     // INTERNAL FUNCTIONS //
 
     impl<T: Config<I>, I: 'static> Pallet<T, I> {
-        pub(super) fn do_claim_membership(idty_id: T::IdtyId, metadata: T::MetaData) -> Weight {
-            let mut total_weight = 1;
-            PendingMembership::<T, I>::remove(&idty_id);
-            total_weight += Self::do_renew_membership_inner(idty_id);
-            Self::deposit_event(Event::MembershipAcquired(idty_id));
-            T::OnEvent::on_event(&sp_membership::Event::MembershipAcquired(idty_id, metadata));
-            total_weight
-        }
         pub(super) fn do_renew_membership(idty_id: T::IdtyId) -> Weight {
             let total_weight = Self::do_renew_membership_inner(idty_id);
             Self::deposit_event(Event::MembershipRenewed(idty_id));
diff --git a/pallets/membership/src/tests.rs b/pallets/membership/src/tests.rs
index 5e28a39e6..183a0d1f8 100644
--- a/pallets/membership/src/tests.rs
+++ b/pallets/membership/src/tests.rs
@@ -67,7 +67,7 @@ fn test_membership_already_acquired() {
         run_to_block(1);
         // Merbership 0 cannot be reclaimed
         assert_eq!(
-            DefaultMembership::claim_membership(Origin::signed(0), 0, ()),
+            DefaultMembership::claim_membership(Origin::signed(0), 0),
             Err(Error::<Test, _>::MembershipAlreadyAcquired.into())
         );
     });
@@ -79,7 +79,7 @@ fn test_membership_request_not_found() {
         run_to_block(1);
         // Merbership 0 cannot be reclaimed
         assert_eq!(
-            DefaultMembership::claim_membership(Origin::signed(1), 1, ()),
+            DefaultMembership::claim_membership(Origin::signed(1), 1),
             Err(Error::<Test, _>::MembershipRequestNotFound.into())
         );
     });
@@ -128,13 +128,17 @@ fn test_membership_revocation() {
         // Membership 0 can't request membership before the end of RevokePeriod (1 + 4 = 5)
         run_to_block(2);
         assert_eq!(
-            DefaultMembership::request_membership(Origin::signed(0), 0),
+            DefaultMembership::request_membership(Origin::signed(0), 0, ()),
             Err(Error::<Test, _>::MembershipRevokedRecently.into())
         );
 
         // Membership 0 can request membership after the end of RevokePeriod (1 + 4 = 5)
         run_to_block(5);
-        assert_ok!(DefaultMembership::request_membership(Origin::signed(0), 0,),);
+        assert_ok!(DefaultMembership::request_membership(
+            Origin::signed(0),
+            0,
+            ()
+        ),);
         assert_eq!(
             System::events()[0].event,
             RuntimeEvent::DefaultMembership(Event::MembershipRequested(0))
@@ -147,7 +151,11 @@ fn test_pending_membership_expiration() {
     new_test_ext(Default::default()).execute_with(|| {
         // Idty 0 request membership
         run_to_block(1);
-        assert_ok!(DefaultMembership::request_membership(Origin::signed(0), 0,),);
+        assert_ok!(DefaultMembership::request_membership(
+            Origin::signed(0),
+            0,
+            ()
+        ),);
         assert_eq!(
             System::events()[0].event,
             RuntimeEvent::DefaultMembership(Event::MembershipRequested(0))
@@ -172,7 +180,11 @@ fn test_membership_workflow() {
     new_test_ext(Default::default()).execute_with(|| {
         // Idty 0 request membership
         run_to_block(1);
-        assert_ok!(DefaultMembership::request_membership(Origin::signed(0), 0,),);
+        assert_ok!(DefaultMembership::request_membership(
+            Origin::signed(0),
+            0,
+            ()
+        ),);
         assert_eq!(
             System::events()[0].event,
             RuntimeEvent::DefaultMembership(Event::MembershipRequested(0))
@@ -180,11 +192,7 @@ fn test_membership_workflow() {
 
         // Then, idty 0 claim membership
         run_to_block(2);
-        assert_ok!(DefaultMembership::claim_membership(
-            Origin::signed(0),
-            0,
-            ()
-        ),);
+        assert_ok!(DefaultMembership::claim_membership(Origin::signed(0), 0,),);
         assert_eq!(
             System::events()[0].event,
             RuntimeEvent::DefaultMembership(Event::MembershipAcquired(0))
diff --git a/runtime/common/src/entities.rs b/runtime/common/src/entities.rs
index 068fd129e..7f676a3b0 100644
--- a/runtime/common/src/entities.rs
+++ b/runtime/common/src/entities.rs
@@ -21,6 +21,13 @@ use scale_info::TypeInfo;
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
 
+#[cfg_attr(feature = "std", derive(Deserialize, Serialize))]
+#[derive(Clone, Encode, Decode, Default, Eq, PartialEq, RuntimeDebug, TypeInfo)]
+pub struct SmithsMembershipMetaData<SessionKeys> {
+    pub peer_id: [u8; 32],
+    pub session_keys: SessionKeys,
+}
+
 #[cfg_attr(feature = "std", derive(Deserialize, Serialize))]
 #[derive(
     Encode, Decode, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, RuntimeDebug, TypeInfo,
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index 401fba2d5..37afe7192 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -14,6 +14,7 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with Substrate-Libre-Currency. If not, see <https://www.gnu.org/licenses/>.
 
+use super::entities::SmithsMembershipMetaData;
 use super::IdtyIndex;
 use frame_support::dispatch::UnfilteredDispatchable;
 use frame_support::instances::Instance2;
@@ -62,19 +63,21 @@ pub struct OnSmithMembershipEventHandler<Inner, Runtime>(
 impl<
         IdtyIndex: Copy + Parameter,
         SessionKeys: Clone,
-        Inner: sp_membership::traits::OnEvent<IdtyIndex, SessionKeys>,
+        Inner: sp_membership::traits::OnEvent<IdtyIndex, SmithsMembershipMetaData<SessionKeys>>,
         Runtime: pallet_identity::Config<IdtyIndex = IdtyIndex>
             + pallet_authority_members::Config<MemberId = IdtyIndex>
-            + pallet_membership::Config<Instance2, MetaData = SessionKeys>
+            + pallet_membership::Config<Instance2, MetaData = SmithsMembershipMetaData<SessionKeys>>
             + pallet_session::Config<Keys = SessionKeys>,
-    > sp_membership::traits::OnEvent<IdtyIndex, SessionKeys>
+    > sp_membership::traits::OnEvent<IdtyIndex, SmithsMembershipMetaData<SessionKeys>>
     for OnSmithMembershipEventHandler<Inner, Runtime>
 {
-    fn on_event(membership_event: &sp_membership::Event<IdtyIndex, SessionKeys>) -> Weight {
+    fn on_event(
+        membership_event: &sp_membership::Event<IdtyIndex, SmithsMembershipMetaData<SessionKeys>>,
+    ) -> Weight {
         (match membership_event {
-            sp_membership::Event::<IdtyIndex, SessionKeys>::MembershipAcquired(
+            sp_membership::Event::MembershipAcquired(
                 idty_index,
-                session_keys,
+                SmithsMembershipMetaData { session_keys, .. },
             ) => {
                 if let Some(idty_value) = pallet_identity::Pallet::<Runtime>::identity(idty_index) {
                     let call = pallet_authority_members::Call::<Runtime>::set_session_keys {
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index 2b151ea50..95a72d14e 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/runtime/common/src/pallets_config.rs
@@ -350,7 +350,7 @@ macro_rules! pallets_config {
 			type IdtyId = IdtyIndex;
 			type MembershipExternalStorage = sp_membership::traits::NoExternalStorage;
 			type MembershipPeriod = SmithMembershipPeriod;
-			type MetaData = opaque::SessionKeys;
+			type MetaData = SmithsMembershipMetaData<opaque::SessionKeys>;
 			type OnEvent = OnSmithMembershipEventHandler<SmithsSubWot, Runtime>;
 			type PendingMembershipPeriod = SmithPendingMembershipPeriod;
 			type RenewablePeriod = SmithRenewablePeriod;
diff --git a/runtime/g1/src/lib.rs b/runtime/g1/src/lib.rs
index 39889d679..3881473fb 100644
--- a/runtime/g1/src/lib.rs
+++ b/runtime/g1/src/lib.rs
@@ -26,8 +26,8 @@ pub mod parameters;
 
 pub use self::parameters::*;
 pub use common_runtime::{
-    constants::*, entities::ValidatorFullIdentification, handlers::*, AccountId, Address, Balance,
-    BlockNumber, FullIdentificationOfImpl, Hash, Header, IdtyIndex, Index, Signature,
+    constants::*, entities::*, handlers::*, AccountId, Address, Balance, BlockNumber,
+    FullIdentificationOfImpl, Hash, Header, IdtyIndex, Index, Signature,
 };
 pub use pallet_balances::Call as BalancesCall;
 pub use pallet_identity::{IdtyStatus, IdtyValue};
diff --git a/runtime/gdev/src/lib.rs b/runtime/gdev/src/lib.rs
index a26e98bc7..7e4b1d26b 100644
--- a/runtime/gdev/src/lib.rs
+++ b/runtime/gdev/src/lib.rs
@@ -26,8 +26,8 @@ pub mod parameters;
 
 pub use self::parameters::*;
 pub use common_runtime::{
-    constants::*, entities::ValidatorFullIdentification, handlers::*, AccountId, Address, Balance,
-    BlockNumber, FullIdentificationOfImpl, Hash, Header, IdtyIndex, Index, Signature,
+    constants::*, entities::*, handlers::*, AccountId, Address, Balance, BlockNumber,
+    FullIdentificationOfImpl, Hash, Header, IdtyIndex, Index, Signature,
 };
 pub use pallet_balances::Call as BalancesCall;
 pub use pallet_duniter_test_parameters::Parameters as GenesisParameters;
diff --git a/runtime/gtest/src/lib.rs b/runtime/gtest/src/lib.rs
index 96b1c9416..3048fd507 100644
--- a/runtime/gtest/src/lib.rs
+++ b/runtime/gtest/src/lib.rs
@@ -26,8 +26,8 @@ pub mod parameters;
 
 pub use self::parameters::*;
 pub use common_runtime::{
-    constants::*, entities::ValidatorFullIdentification, handlers::*, AccountId, Address, Balance,
-    BlockNumber, FullIdentificationOfImpl, Hash, Header, IdtyIndex, Index, Signature,
+    constants::*, entities::*, handlers::*, AccountId, Address, Balance, BlockNumber,
+    FullIdentificationOfImpl, Hash, Header, IdtyIndex, Index, Signature,
 };
 pub use pallet_balances::Call as BalancesCall;
 pub use pallet_identity::{IdtyStatus, IdtyValue};
-- 
GitLab