diff --git a/pallets/authority-members/src/benchmarking.rs b/pallets/authority-members/src/benchmarking.rs index d19e73ccf7e90d7de663834db006a19b64c4b146..c0dcc62b8c275b962fd95ccc3be3592bf67ae0f5 100644 --- a/pallets/authority-members/src/benchmarking.rs +++ b/pallets/authority-members/src/benchmarking.rs @@ -59,7 +59,7 @@ benchmarks! { let caller: T::AccountId = Members::<T>::get(id).unwrap().owner_key; let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); let validator_id = T::ValidatorIdOf::convert(caller.clone()).unwrap(); - let session_keys: T::KeysWrapper = pallet_session::NextKeys::<T>::get(validator_id).unwrap().into(); + let session_keys: T::Keys = pallet_session::NextKeys::<T>::get(validator_id).unwrap().into(); }: _<T::RuntimeOrigin>(caller_origin, session_keys) remove_member { let id: T::MemberId = OnlineAuthorities::<T>::get()[0]; diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs index f5d86823d701a7fe5d26b0d983ace1bc139ae408..7884482b31a30a650fc32aa341ef53a0b9a93f1e 100644 --- a/pallets/authority-members/src/lib.rs +++ b/pallets/authority-members/src/lib.rs @@ -66,7 +66,6 @@ pub mod pallet { pub trait Config: frame_system::Config + pallet_session::Config + pallet_session::historical::Config { - type KeysWrapper: Parameter + Into<Self::Keys> + From<Self::Keys>; type IsMember: IsMember<Self::MemberId>; type OnNewSession: OnNewSession; type OnRemovedMember: OnRemovedMember<Self::MemberId>; @@ -295,15 +294,12 @@ pub mod pallet { /// declare new session keys to replace current ones #[pallet::call_index(2)] #[pallet::weight(<T as pallet::Config>::WeightInfo::set_session_keys())] - pub fn set_session_keys( - origin: OriginFor<T>, - keys: T::KeysWrapper, - ) -> DispatchResultWithPostInfo { + pub fn set_session_keys(origin: OriginFor<T>, keys: T::Keys) -> DispatchResultWithPostInfo { let who = ensure_signed(origin.clone())?; let member_id = Self::verify_ownership_and_membership(&who)?; let _post_info = pallet_session::Call::<T>::set_keys { - keys: keys.into(), + keys, proof: vec![], } .dispatch_bypass_filter(origin)?; diff --git a/pallets/authority-members/src/mock.rs b/pallets/authority-members/src/mock.rs index c5cfce266b289da915dad85c7ec45f1420b09344..d04f0f47f89905eb13f211dcfd58098887743239 100644 --- a/pallets/authority-members/src/mock.rs +++ b/pallets/authority-members/src/mock.rs @@ -151,7 +151,6 @@ impl IsMember<u64> for TestIsSmithMember { } impl pallet_authority_members::Config for Test { - type KeysWrapper = MockSessionKeys; type IsMember = TestIsSmithMember; type MaxAuthorities = ConstU32<4>; type MemberId = u64; diff --git a/pallets/distance/src/mock.rs b/pallets/distance/src/mock.rs index 977c4228c063e0a65c460dd95c404eaa04b2edc7..5d580b13782a6b461c8b6154051b18566a1cafc5 100644 --- a/pallets/distance/src/mock.rs +++ b/pallets/distance/src/mock.rs @@ -180,7 +180,6 @@ impl<T: pallet_identity::Config> sp_runtime::traits::Convert<T::AccountId, Optio } impl pallet_authority_members::Config for Test { - type KeysWrapper = MockSessionKeys; type IsMember = TestIsSmithMember; type MaxAuthorities = ConstU32<4>; type MemberId = u32; diff --git a/pallets/duniter-wot/src/lib.rs b/pallets/duniter-wot/src/lib.rs index 1b3b9bdb6237a2efd4a48c0e3d2bc8ec451c50e0..eb9fc59e0f482854722169cdccc5431668f835bb 100644 --- a/pallets/duniter-wot/src/lib.rs +++ b/pallets/duniter-wot/src/lib.rs @@ -127,8 +127,8 @@ pub mod pallet { NotAllowedToRemoveIdty, /// Issuer can not emit cert because it is not validated IssuerCanNotEmitCert, - /// Can not issue cert to unconfirmed identity - CertToUnconfirmedIdty, + /// Can not issue cert to identity without membership or pending membership + CertToUndefined, /// Issuer or receiver not found IdtyNotFound, } @@ -164,11 +164,9 @@ where fn check_confirm_identity(idty_index: IdtyIndex) -> Result<(), DispatchError> { // main WoT automatic action if !T::IsSubWot::get() { - pallet_membership::Pallet::<T, I>::force_request_membership( - idty_index, - Default::default(), - ) - .map_err(|e| e.error)?; + // force add a membership request to the main WoT + pallet_membership::Pallet::<T, I>::force_request_membership(idty_index) + .map_err(|e| e.error)?; } // no constraints for subwot Ok(()) @@ -209,12 +207,16 @@ where // implement cert call checks impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<IdtyIndex> for Pallet<T, I> +// TODO add the following where clause once checks can be done on pallet instance +// where +// T: pallet_membership::Config<I>, { // check the following: // - issuer has identity // - issuer identity is validated // - receiver has identity // - receiver identity is confirmed or validated + // - receiver has membership // // /!\ do not check the following: // - receiver has membership @@ -224,7 +226,13 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id // - issuer can issue cert even if he lost his membership // (not renewed or passed below cert threshold and above again without claiming membership) // this is counterintuitive behavior but not a big problem + // + // TODO to fix this strange behavior, we will have to make the tests + // (CheckCertAllowed and CheckMembershipCallAllowed) run on the relevant instance + // i.e. Cert for Wot, SmithCert for SmithWot... + // → see issue #136 fn check_cert_allowed(issuer: IdtyIndex, receiver: IdtyIndex) -> Result<(), DispatchError> { + // issuer checks // ensure issuer has validated identity if let Some(issuer_data) = pallet_identity::Pallet::<T>::identity(issuer) { ensure!( @@ -234,15 +242,30 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::CheckCertAllowed<Id } else { return Err(Error::<T, I>::IdtyNotFound.into()); } + // issue #136 this has to be done on the correct instance of membership pallet + // // ensure issuer has membership + // if pallet_membership::Pallet::<T, I>::membership(issuer).is_none() { + // // improvement: give reason why issuer can not emit cert (not member) + // return Err(Error::<T, I>::IssuerCanNotEmitCert.into()); + // } + + // receiver checks // ensure receiver has confirmed or validated identity if let Some(receiver_data) = pallet_identity::Pallet::<T>::identity(receiver) { match receiver_data.status { IdtyStatus::ConfirmedByOwner | IdtyStatus::Validated => {} // able to receive cert - IdtyStatus::Created => return Err(Error::<T, I>::CertToUnconfirmedIdty.into()), + IdtyStatus::Created => return Err(Error::<T, I>::CertToUndefined.into()), }; } else { return Err(Error::<T, I>::IdtyNotFound.into()); } + // issue #136 this has to be done on the correct instance of membership pallet + // // ensure receiver has a membership or a pending membership + // if pallet_membership::Pallet::<T, I>::pending_membership(issuer).is_none() + // && pallet_membership::Pallet::<T, I>::membership(issuer).is_none() + // { + // return Err(Error::<T, I>::CertToUndefined.into()); + // } Ok(()) } } @@ -299,31 +322,30 @@ impl<T: Config<I>, I: 'static> sp_membership::traits::CheckMembershipCallAllowed } // implement membership event handler -impl<T: Config<I>, I: 'static, MetaData> sp_membership::traits::OnEvent<IdtyIndex, MetaData> - for Pallet<T, I> +impl<T: Config<I>, I: 'static> sp_membership::traits::OnEvent<IdtyIndex> for Pallet<T, I> where - T: pallet_membership::Config<I, MetaData = MetaData>, + T: pallet_membership::Config<I>, { - fn on_event(membership_event: &sp_membership::Event<IdtyIndex, MetaData>) -> Weight { + fn on_event(membership_event: &sp_membership::Event<IdtyIndex>) -> Weight { match membership_event { - sp_membership::Event::<IdtyIndex, MetaData>::MembershipAcquired(idty_index, _) => { + sp_membership::Event::<IdtyIndex>::MembershipAcquired(idty_index) => { if !T::IsSubWot::get() { // when membership is acquired, validate identity // (only used on first membership acquiry) pallet_identity::Pallet::<T>::try_validate_identity(*idty_index); } } - sp_membership::Event::<IdtyIndex, MetaData>::MembershipExpired(_) => {} + sp_membership::Event::<IdtyIndex>::MembershipExpired(_) => {} // Membership revocation cases: // - Triggered by main identity removal: the underlying identity will be removed by the // caller. // - Triggered by the membership pallet: it's only possible for a sub-wot, so we // should not remove the underlying identity // So, in any case, we must do nothing - sp_membership::Event::<IdtyIndex, MetaData>::MembershipRevoked(_) => {} - sp_membership::Event::<IdtyIndex, MetaData>::MembershipRenewed(_) => {} - sp_membership::Event::<IdtyIndex, MetaData>::MembershipRequested(_) => {} - sp_membership::Event::<IdtyIndex, MetaData>::PendingMembershipExpired(idty_index) => { + sp_membership::Event::<IdtyIndex>::MembershipRevoked(_) => {} + sp_membership::Event::<IdtyIndex>::MembershipRenewed(_) => {} + sp_membership::Event::<IdtyIndex>::MembershipRequested(_) => {} + sp_membership::Event::<IdtyIndex>::PendingMembershipExpired(idty_index) => { Self::dispatch_idty_call(pallet_identity::Call::remove_identity { idty_index: *idty_index, idty_name: None, diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index cd25f68a9e096f13024444a13d6b82271757321f..29454df22851cd21301790b099cb650adec2c7d7 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -151,7 +151,6 @@ impl pallet_membership::Config<Instance1> for Test { type IdtyIdOf = IdentityIndexOf<Self>; type AccountIdOf = (); type MembershipPeriod = MembershipPeriod; - type MetaData = (); type OnEvent = DuniterWot; type RuntimeEvent = RuntimeEvent; type WeightInfo = (); @@ -209,7 +208,6 @@ impl pallet_membership::Config<Instance2> for Test { type IdtyIdOf = IdentityIndexOf<Self>; type AccountIdOf = (); type MembershipPeriod = SmithMembershipPeriod; - type MetaData = (); type OnEvent = SmithSubWot; type PendingMembershipPeriod = SmithPendingMembershipPeriod; type WeightInfo = (); diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs index 6cefede1f13db6e67eedb9ed7efec3f694af8f7b..24d0a2c148ab651b50368564fc0ae709d3ead5d7 100644 --- a/pallets/duniter-wot/src/tests.rs +++ b/pallets/duniter-wot/src/tests.rs @@ -79,10 +79,9 @@ fn test_join_smiths() { run_to_block(2); // Dave shoud be able to request smith membership - assert_ok!(SmithMembership::request_membership( - RuntimeOrigin::signed(4), - () - )); + assert_ok!(SmithMembership::request_membership(RuntimeOrigin::signed( + 4 + ),)); System::assert_has_event(RuntimeEvent::SmithMembership( pallet_membership::Event::MembershipRequested(4), )); diff --git a/pallets/membership/src/benchmarking.rs b/pallets/membership/src/benchmarking.rs index da0ce28b79f6aa29453b8b379843f268c982865c..11c95849efa4196bd17807b37e3fd6c377b68ae7 100644 --- a/pallets/membership/src/benchmarking.rs +++ b/pallets/membership/src/benchmarking.rs @@ -46,7 +46,7 @@ benchmarks_instance_pallet! { let caller: T::AccountId = T::AccountIdOf::convert(idty.clone()).unwrap(); let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); // Lazily prepare call as this extrinsic will always return an errror when in subwot - let call = Call::<T, I>::request_membership { metadata: T::MetaData ::default()}; + let call = Call::<T, I>::request_membership { }; }: { call.dispatch_bypass_filter(caller_origin).ok(); } @@ -58,7 +58,7 @@ benchmarks_instance_pallet! { claim_membership { let idty: T::IdtyId = 3.into(); Membership::<T, I>::take(idty); - PendingMembership::<T, I>::insert(idty.clone(), T::MetaData::default()); + PendingMembership::<T, I>::insert(idty.clone(), ()); let caller: T::AccountId = T::AccountIdOf::convert(idty.clone()).unwrap(); let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); T::BenchmarkSetupHandler::force_status_ok(&idty, &caller); diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs index 1b7a2d07e017db47a75e68955e81beeeee9edd85..3855bf927d51cfb435f7bc9f6de0f0c6cc4a8bc4 100644 --- a/pallets/membership/src/lib.rs +++ b/pallets/membership/src/lib.rs @@ -81,13 +81,11 @@ pub mod pallet { type IdtyIdOf: Convert<Self::AccountId, Option<Self::IdtyId>>; /// Something that gives the AccountId of an IdtyId type AccountIdOf: Convert<Self::IdtyId, Option<Self::AccountId>>; - /// Optional metadata - type MetaData: Default + Parameter + Validate<Self::AccountId>; #[pallet::constant] /// Maximum life span of a non-renewable membership (in number of blocks) type MembershipPeriod: Get<Self::BlockNumber>; /// On event handler - type OnEvent: OnEvent<Self::IdtyId, Self::MetaData>; + type OnEvent: OnEvent<Self::IdtyId>; #[pallet::constant] /// Maximum period (in number of blocks), where an identity can remain pending subscription. type PendingMembershipPeriod: Get<Self::BlockNumber>; @@ -140,11 +138,11 @@ pub mod pallet { pub type MembershipsExpireOn<T: Config<I>, I: 'static = ()> = StorageMap<_, Twox64Concat, T::BlockNumber, Vec<T::IdtyId>, ValueQuery>; - /// maps identity id to pending membership metadata + /// identities with pending membership request #[pallet::storage] #[pallet::getter(fn pending_membership)] pub type PendingMembership<T: Config<I>, I: 'static = ()> = - StorageMap<_, Twox64Concat, T::IdtyId, T::MetaData, OptionQuery>; + StorageMap<_, Twox64Concat, T::IdtyId, (), OptionQuery>; /// maps block number to the list of memberships set to expire at this block #[pallet::storage] @@ -181,8 +179,6 @@ pub mod pallet { #[pallet::error] pub enum Error<T, I = ()> { - /// Invalid meta data - InvalidMetaData, /// Identity id not found IdtyIdNotFound, /// Membership already acquired @@ -218,19 +214,14 @@ pub mod pallet { /// (only available for sub wot, automatic for main wot) #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::request_membership())] - pub fn request_membership( - origin: OriginFor<T>, - metadata: T::MetaData, - ) -> DispatchResultWithPostInfo { + pub fn request_membership(origin: OriginFor<T>) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let idty_id = T::IdtyIdOf::convert(who.clone()).ok_or(Error::<T, I>::IdtyIdNotFound)?; - if !metadata.validate(&who) { - return Err(Error::<T, I>::InvalidMetaData.into()); - } + T::CheckMembershipCallAllowed::check_idty_allowed_to_request_membership(&idty_id)?; - Self::do_request_membership(idty_id, metadata) + Self::do_request_membership(idty_id) } /// claim membership @@ -288,11 +279,8 @@ pub mod pallet { impl<T: Config<I>, I: 'static> Pallet<T, I> { /// force request membership - pub fn force_request_membership( - idty_id: T::IdtyId, - metadata: T::MetaData, - ) -> DispatchResultWithPostInfo { - Self::do_request_membership(idty_id, metadata) + pub fn force_request_membership(idty_id: T::IdtyId) -> DispatchResultWithPostInfo { + Self::do_request_membership(idty_id) } /// force expire membership @@ -336,10 +324,7 @@ pub mod pallet { } /// perform the membership request - fn do_request_membership( - idty_id: T::IdtyId, - metadata: T::MetaData, - ) -> DispatchResultWithPostInfo { + fn do_request_membership(idty_id: T::IdtyId) -> DispatchResultWithPostInfo { // checks if PendingMembership::<T, I>::contains_key(idty_id) { return Err(Error::<T, I>::MembershipAlreadyRequested.into()); @@ -352,7 +337,7 @@ pub mod pallet { let expire_on = block_number + T::PendingMembershipPeriod::get(); // apply membership request - PendingMembership::<T, I>::insert(idty_id, metadata); + PendingMembership::<T, I>::insert(idty_id, ()); 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)); @@ -371,10 +356,10 @@ pub mod pallet { /// perform membership claim fn do_claim_membership(idty_id: T::IdtyId) { - if let Some(metadata) = PendingMembership::<T, I>::take(idty_id) { + if PendingMembership::<T, I>::take(idty_id).is_some() { Self::insert_membership_and_schedule_expiry(idty_id); Self::deposit_event(Event::MembershipAcquired(idty_id)); - T::OnEvent::on_event(&sp_membership::Event::MembershipAcquired(idty_id, metadata)); + T::OnEvent::on_event(&sp_membership::Event::MembershipAcquired(idty_id)); } // else { unreachable if check_allowed_to_claim called before } } @@ -388,11 +373,11 @@ pub mod pallet { } } - /// perform mebership expiration + /// perform membership expiration // add pending membership and schedule expiry of pending membership fn do_expire_membership(idty_id: T::IdtyId, expire_on: T::BlockNumber) -> Weight { if Membership::<T, I>::take(idty_id).is_some() { - PendingMembership::<T, I>::insert(idty_id, T::MetaData::default()); + PendingMembership::<T, I>::insert(idty_id, ()); PendingMembershipsExpireOn::<T, I>::append(expire_on, idty_id); } // else should not happen diff --git a/pallets/membership/src/mock.rs b/pallets/membership/src/mock.rs index d97f1f86ab0e480c6a3fa439b11af5a7939f8f18..74f44e85bf4ba6c1cabbe70808181320dc3901f3 100644 --- a/pallets/membership/src/mock.rs +++ b/pallets/membership/src/mock.rs @@ -88,7 +88,6 @@ impl pallet_membership::Config for Test { type IdtyIdOf = ConvertInto; type AccountIdOf = ConvertInto; type MembershipPeriod = MembershipPeriod; - type MetaData = (); type OnEvent = (); type PendingMembershipPeriod = PendingMembershipPeriod; type RuntimeEvent = RuntimeEvent; diff --git a/pallets/membership/src/tests.rs b/pallets/membership/src/tests.rs index f11ab9680a18bc8206a4bb5d8d114781b7235b93..a899b4424c20ce2d1b384ad866639e190c5c0e32 100644 --- a/pallets/membership/src/tests.rs +++ b/pallets/membership/src/tests.rs @@ -148,7 +148,6 @@ fn test_membership_revocation() { run_to_block(5); assert_ok!(DefaultMembership::request_membership( RuntimeOrigin::signed(0), - () )); System::assert_has_event(RtEvent::DefaultMembership(Event::MembershipRequested(0))); }); @@ -162,7 +161,6 @@ fn test_pending_membership_expiration() { run_to_block(1); assert_ok!(DefaultMembership::request_membership( RuntimeOrigin::signed(0), - () )); System::assert_has_event(RtEvent::DefaultMembership(Event::MembershipRequested(0))); @@ -191,7 +189,6 @@ fn test_membership_workflow() { run_to_block(1); assert_ok!(DefaultMembership::request_membership( RuntimeOrigin::signed(0), - () )); System::assert_has_event(RtEvent::DefaultMembership(Event::MembershipRequested(0))); diff --git a/primitives/membership/src/lib.rs b/primitives/membership/src/lib.rs index 2d74c62309df4098609ba1ce21eaf8f6dc3f5735..f59fcde5de2c1e15145eea6ecb18bfdb350a73ec 100644 --- a/primitives/membership/src/lib.rs +++ b/primitives/membership/src/lib.rs @@ -27,9 +27,9 @@ use scale_info::TypeInfo; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; -pub enum Event<IdtyId, MetaData = ()> { +pub enum Event<IdtyId> { /// A membership has acquired - MembershipAcquired(IdtyId, MetaData), + MembershipAcquired(IdtyId), /// A membership has expired MembershipExpired(IdtyId), /// A membership has renewed diff --git a/primitives/membership/src/traits.rs b/primitives/membership/src/traits.rs index 0480a64f38aff07a12448a012e7860fbcd82c6a8..2b68516ca4af8d6d0700f3b2c36ceec038ed3764 100644 --- a/primitives/membership/src/traits.rs +++ b/primitives/membership/src/traits.rs @@ -38,12 +38,12 @@ pub trait IsInPendingMemberships<IdtyId> { fn is_in_pending_memberships(idty_id: IdtyId) -> bool; } -pub trait OnEvent<IdtyId, MetaData> { - fn on_event(event: &crate::Event<IdtyId, MetaData>) -> Weight; +pub trait OnEvent<IdtyId> { + fn on_event(event: &crate::Event<IdtyId>) -> Weight; } -impl<IdtyId, MetaData> OnEvent<IdtyId, MetaData> for () { - fn on_event(_: &crate::Event<IdtyId, MetaData>) -> Weight { +impl<IdtyId> OnEvent<IdtyId> for () { + fn on_event(_: &crate::Event<IdtyId>) -> Weight { Weight::zero() } } diff --git a/resources/metadata.scale b/resources/metadata.scale index 2680c3b02a4ca74e19ed66cc13eb88883db14aef..157b25e4f0edd9fcfa7acfd37ac5e71a25f7c187 100644 Binary files a/resources/metadata.scale and b/resources/metadata.scale differ diff --git a/runtime/common/src/entities.rs b/runtime/common/src/entities.rs index 823ae2a71c74fe098e3334998c2065c39f4d7770..7ee895b7fa4c00058470756c06912ceeb296d4b0 100644 --- a/runtime/common/src/entities.rs +++ b/runtime/common/src/entities.rs @@ -14,7 +14,6 @@ // You should have received a copy of the GNU Affero General Public License // along with Duniter-v2S. If not, see <https://www.gnu.org/licenses/>. -use super::AccountId; use frame_support::pallet_prelude::*; use scale_info::TypeInfo; #[cfg(feature = "std")] @@ -34,40 +33,6 @@ macro_rules! declare_session_keys { pub authority_discovery: AuthorityDiscovery, } } - - #[derive(Clone, codec::Decode, Debug, codec::Encode, Eq, PartialEq)] - pub struct SessionKeysWrapper(pub SessionKeys); - - impl From<SessionKeysWrapper> for SessionKeys { - fn from(keys_wrapper: SessionKeysWrapper) -> SessionKeys { - keys_wrapper.0 - } - } - impl From<SessionKeys> for SessionKeysWrapper { - fn from(session_keys: SessionKeys) -> SessionKeysWrapper { - SessionKeysWrapper(session_keys) - } - } - - impl scale_info::TypeInfo for SessionKeysWrapper { - type Identity = [u8; 128]; - - fn type_info() -> scale_info::Type { - Self::Identity::type_info() - } - } - - // Dummy implementation only for benchmarking - impl Default for SessionKeysWrapper { - fn default() -> Self { - SessionKeysWrapper(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(), - }) - } - } } } } @@ -94,42 +59,6 @@ impl From<IdtyData> for pallet_universal_dividend::FirstEligibleUd { } } -#[cfg_attr(feature = "std", derive(Deserialize, Serialize))] -#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo)] -pub struct SmithMembershipMetaData<SessionKeysWrapper> { - pub owner_key: AccountId, - pub p2p_endpoint: sp_runtime::RuntimeString, - pub session_keys: SessionKeysWrapper, -} - -impl<SessionKeysWrapper: Default> Default for SmithMembershipMetaData<SessionKeysWrapper> { - #[cfg(not(feature = "runtime-benchmarks"))] - fn default() -> Self { - unreachable!() - } - #[cfg(feature = "runtime-benchmarks")] - // dummy implementation for benchmarking - fn default() -> Self { - SmithMembershipMetaData { - owner_key: AccountId::from([ - // Dave (FIXME avoid stupid metadata) - 48, 103, 33, 33, 29, 84, 4, 189, 157, 168, 142, 2, 4, 54, 10, 26, 154, 184, 184, - 124, 102, 193, 188, 47, 205, 211, 127, 60, 34, 34, 204, 32, - ]), - p2p_endpoint: sp_runtime::RuntimeString::default(), - session_keys: SessionKeysWrapper::default(), - } - } -} - -impl<SessionKeysWrapper> sp_membership::traits::Validate<AccountId> - for SmithMembershipMetaData<SessionKeysWrapper> -{ - fn validate(&self, who: &AccountId) -> bool { - &self.owner_key == who - } -} - #[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 e05a97d4fd88fbb2be9f359cee76934b324ff7b4..540054ab341379c3410bf2a030835a6290a803df 100644 --- a/runtime/common/src/handlers.rs +++ b/runtime/common/src/handlers.rs @@ -71,14 +71,14 @@ where // membership event runtime handler pub struct OnMembershipEventHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>); impl< - Inner: sp_membership::traits::OnEvent<IdtyIndex, ()>, + Inner: sp_membership::traits::OnEvent<IdtyIndex>, Runtime: frame_system::Config<AccountId = AccountId> + pallet_identity::Config<IdtyData = IdtyData, IdtyIndex = IdtyIndex> - + pallet_membership::Config<Instance1, MetaData = ()> + + pallet_membership::Config<Instance1> + pallet_universal_dividend::Config, - > sp_membership::traits::OnEvent<IdtyIndex, ()> for OnMembershipEventHandler<Inner, Runtime> + > sp_membership::traits::OnEvent<IdtyIndex> for OnMembershipEventHandler<Inner, Runtime> { - fn on_event(membership_event: &sp_membership::Event<IdtyIndex, ()>) -> Weight { + fn on_event(membership_event: &sp_membership::Event<IdtyIndex>) -> Weight { (match membership_event { // when membership is removed, call on_removed_member handler which auto claims UD sp_membership::Event::MembershipRevoked(idty_index) @@ -97,7 +97,7 @@ impl< } } // when main membership is acquired, it starts getting right to UD - sp_membership::Event::MembershipAcquired(idty_index, _) => { + sp_membership::Event::MembershipAcquired(idty_index) => { pallet_identity::Identities::<Runtime>::mutate_exists(idty_index, |idty_val_opt| { if let Some(ref mut idty_val) = idty_val_opt { idty_val.data = IdtyData { @@ -123,43 +123,18 @@ pub struct OnSmithMembershipEventHandler<Inner, Runtime>( ); impl< IdtyIndex: Copy + Parameter, - SessionKeysWrapper: Clone, - Inner: sp_membership::traits::OnEvent<IdtyIndex, SmithMembershipMetaData<SessionKeysWrapper>>, + Inner: sp_membership::traits::OnEvent<IdtyIndex>, Runtime: frame_system::Config<AccountId = AccountId> + pallet_identity::Config<IdtyIndex = IdtyIndex> - + pallet_authority_members::Config<KeysWrapper = SessionKeysWrapper, MemberId = IdtyIndex> - + pallet_membership::Config< - Instance2, - MetaData = SmithMembershipMetaData<SessionKeysWrapper>, - >, - > sp_membership::traits::OnEvent<IdtyIndex, SmithMembershipMetaData<SessionKeysWrapper>> - for OnSmithMembershipEventHandler<Inner, Runtime> + + pallet_authority_members::Config<MemberId = IdtyIndex> + + pallet_membership::Config<Instance2>, + > sp_membership::traits::OnEvent<IdtyIndex> for OnSmithMembershipEventHandler<Inner, Runtime> { - fn on_event( - membership_event: &sp_membership::Event< - IdtyIndex, - SmithMembershipMetaData<SessionKeysWrapper>, - >, - ) -> Weight { + fn on_event(membership_event: &sp_membership::Event<IdtyIndex>) -> Weight { (match membership_event { - sp_membership::Event::MembershipAcquired( - _idty_index, - SmithMembershipMetaData { - owner_key, - session_keys, - .. - }, - ) => { - let call = pallet_authority_members::Call::<Runtime>::set_session_keys { - keys: session_keys.clone(), - }; - if let Err(e) = call.dispatch_bypass_filter( - frame_system::Origin::<Runtime>::Signed(owner_key.clone()).into(), - ) { - sp_std::if_std! { - println!("fail to set session keys: {:?}", e) - } - } + sp_membership::Event::MembershipAcquired(_idty_index) => { + // nothing when smith membership acquired + // user will have to claim authority membership Weight::zero() } sp_membership::Event::MembershipRevoked(idty_index) => { diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index 6b49541287f0f7464331fe1ce327ab6138cde430..80efaf60f4adc8744cc9afe816af38bb14d8580f 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -236,7 +236,6 @@ macro_rules! pallets_config { } impl pallet_authority_members::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type KeysWrapper = opaque::SessionKeysWrapper; type IsMember = SmithMembership; type OnNewSession = OnNewSessionHandler<Runtime>; type OnRemovedMember = OnRemovedAuthorityMemberHandler<Runtime>; @@ -488,7 +487,6 @@ macro_rules! pallets_config { type IdtyIdOf = common_runtime::providers::IdentityIndexOf<Self>; type AccountIdOf = common_runtime::providers::IdentityAccountIdProvider<Self>; type MembershipPeriod = MembershipPeriod; - type MetaData = (); type OnEvent = OnMembershipEventHandler<Wot, Runtime>; type PendingMembershipPeriod = PendingMembershipPeriod; type RuntimeEvent = RuntimeEvent; @@ -538,7 +536,6 @@ macro_rules! pallets_config { type IdtyIdOf = common_runtime::providers::IdentityIndexOf<Self>; type AccountIdOf = common_runtime::providers::IdentityAccountIdProvider<Self>; type MembershipPeriod = SmithMembershipPeriod; - type MetaData = SmithMembershipMetaData<opaque::SessionKeysWrapper>; type OnEvent = OnSmithMembershipEventHandler<SmithSubWot, Runtime>; type PendingMembershipPeriod = SmithPendingMembershipPeriod; type RuntimeEvent = RuntimeEvent; diff --git a/runtime/gdev/tests/fixme_tests.rs b/runtime/gdev/tests/fixme_tests.rs new file mode 100644 index 0000000000000000000000000000000000000000..28b9777e86daaed4cc4a8e04f29ca0be2f348743 --- /dev/null +++ b/runtime/gdev/tests/fixme_tests.rs @@ -0,0 +1,75 @@ +// Copyright 2021 Axiom-Team +// +// This file is part of Duniter-v2S. +// +// Duniter-v2S is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, version 3 of the License. +// +// Duniter-v2S is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with Duniter-v2S. If not, see <https://www.gnu.org/licenses/>. + +// these integration tests show current behavior that is counter-intuitive or outside specs +// they should failed after the related issue is fixed + +mod common; + +use common::*; +use frame_support::assert_ok; +use gdev_runtime::*; +use sp_keyring::AccountKeyring; + +/// issue #136 +/// a smith should not be able to add a smith cert to an +/// identity who has no smith membership or pending membership +#[test] +fn can_add_smith_cert_without_pending_membership_or_membership() { + // 3 smith (1. Alice, 2. Bob, 3. Charlie) + // 4 identities (4. Dave) + ExtBuilder::new(1, 3, 4).build().execute_with(|| { + run_to_block(1); + + // FIXME Bob can add new smith cert to to dave even he did not requested smith membership + assert_ok!(SmithCert::add_cert( + frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into(), + 2, // bob + 4 // dave + )); + }); +} + +/// issue #136 +/// an identity should not be able to add cert +/// when its membership is suspended +#[test] +fn can_still_issue_cert_when_membership_lost() { + ExtBuilder::new(1, 3, 4).build().execute_with(|| { + run_to_block(1); + + // expire Bob membership (could happen for negative distance evaluation for example) + assert_ok!(Membership::force_expire_membership( + 2, // Bob + )); + System::assert_has_event(RuntimeEvent::Membership( + pallet_membership::Event::MembershipExpired(2), + )); + + // FIXME this should not be possible + assert_ok!(Cert::add_cert( + frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into(), + 2, // Bob + 3, // Charlie + )); + System::assert_has_event(RuntimeEvent::Cert( + pallet_certification::Event::RenewedCert { + issuer: 2, + receiver: 3, + }, + )); + }); +} diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs index b0525631426e8af64849354f58e5a3d75dd0af12..982a7ccc9077ad293b2588a96e8135f84309d854 100644 --- a/runtime/gdev/tests/integration_tests.rs +++ b/runtime/gdev/tests/integration_tests.rs @@ -676,14 +676,6 @@ fn test_smith_certification() { 2 // bob )); - // THIS IS STRANGE BEHAVIOR - // bob can add new smith cert to to dave even he did not requested smith membership - assert_ok!(SmithCert::add_cert( - frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into(), - 2, // bob - 4 // dave - )); - // charlie can not add new cert to eve (no identity) assert_noop!( SmithCert::add_cert( @@ -697,6 +689,64 @@ fn test_smith_certification() { }); } +/// test the full process to join smith from main wot member to authority member +#[test] +fn test_smith_process() { + ExtBuilder::new(1, 3, 4).with_initial_balances(vec![(AccountKeyring::Dave.to_account_id(), 1_000)]) + .build().execute_with(|| { + run_to_block(1); + + let alice = AccountKeyring::Alice.to_account_id(); + let bob = AccountKeyring::Bob.to_account_id(); + let charlie = AccountKeyring::Charlie.to_account_id(); + + // Eve can not request smith membership because not member of the smith wot + assert_noop!(SmithMembership::request_membership( + frame_system::RawOrigin::Signed(AccountKeyring::Eve.to_account_id()).into()), + pallet_membership::Error::<gdev_runtime::Runtime, pallet_membership::Instance2>::IdtyIdNotFound, + ); + + // Dave can request smith membership (currently optional) + assert_ok!(SmithMembership::request_membership( + frame_system::RawOrigin::Signed(AccountKeyring::Dave.to_account_id()).into(), + )); + + assert_eq!(SmithMembership::pending_membership(5), None); // none for Eve + assert_eq!(SmithMembership::pending_membership(4), Some(())); // Dave + + // then Alice Bob and Charlie can certify Dave + assert_ok!(SmithCert::add_cert(frame_system::RawOrigin::Signed(alice).into(), 1, 4)); + assert_ok!(SmithCert::add_cert(frame_system::RawOrigin::Signed(bob).into(), 2, 4)); + assert_ok!(SmithCert::add_cert(frame_system::RawOrigin::Signed(charlie).into(), 3, 4)); + + // with these three smith certs, Dave can claim membership + assert_ok!(SmithMembership::claim_membership( + frame_system::RawOrigin::Signed(AccountKeyring::Dave.to_account_id()).into(), + )); + + // Dave is then member of the smith wot + assert_eq!(SmithMembership::membership(4), Some(sp_membership::MembershipData { + expire_on: 1001, // 1 + 1000 + })); + + // Dave can set his (dummy) session keys + assert_ok!(AuthorityMembers::set_session_keys( + frame_system::RawOrigin::Signed(AccountKeyring::Dave.to_account_id()).into(), + 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(), + } + )); + + // Dave can go online + assert_ok!(AuthorityMembers::go_online( + frame_system::RawOrigin::Signed(AccountKeyring::Dave.to_account_id()).into(), + )); + }) +} + /// test create new account with balance lower than existential deposit // the treasury gets the dust #[test]