From 8523a163c50cf9b58072542cd257962db86f9c09 Mon Sep 17 00:00:00 2001 From: bgallois <benjamin@gallois.cc> Date: Wed, 11 Sep 2024 14:58:26 +0200 Subject: [PATCH] fix https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/245 --- pallets/distance/src/mock.rs | 1 + pallets/duniter-wot/src/mock.rs | 1 + pallets/identity/src/lib.rs | 11 +++++++++++ pallets/identity/src/mock.rs | 1 + pallets/identity/src/traits.rs | 12 ++++++++++++ pallets/quota/src/mock.rs | 1 + resources/metadata.scale | Bin 149703 -> 149769 bytes runtime/common/src/handlers.rs | 14 +++++++++++++ runtime/common/src/pallets_config.rs | 1 + runtime/gdev/tests/integration_tests.rs | 25 ++++++++++++++++++++++++ 10 files changed, 67 insertions(+) diff --git a/pallets/distance/src/mock.rs b/pallets/distance/src/mock.rs index edcfedbfe..f837bac85 100644 --- a/pallets/distance/src/mock.rs +++ b/pallets/distance/src/mock.rs @@ -243,6 +243,7 @@ impl pallet_identity::Config for Test { type IdtyNameValidator = IdtyNameValidatorTestImpl; type OnNewIdty = (); type OnRemoveIdty = (); + type OwnerKeyChangePermission = (); type RuntimeEvent = RuntimeEvent; type Signature = TestSignature; type Signer = UintAuthorityId; diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs index 9ebef148c..ddb4bf63f 100644 --- a/pallets/duniter-wot/src/mock.rs +++ b/pallets/duniter-wot/src/mock.rs @@ -123,6 +123,7 @@ impl pallet_identity::Config for Test { type IdtyNameValidator = IdtyNameValidatorTestImpl; type OnNewIdty = DuniterWot; type OnRemoveIdty = DuniterWot; + type OwnerKeyChangePermission = (); type RuntimeEvent = RuntimeEvent; type Signature = TestSignature; type Signer = UintAuthorityId; diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index f0f70e6c5..f09c2d31c 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -132,6 +132,9 @@ pub mod pallet { /// The type used to check account worthiness. type CheckAccountWorthiness: CheckAccountWorthiness<Self>; + /// Handler that checks the necessary permissions for an identity's owner key change. + type OwnerKeyChangePermission: CheckKeyChangeAllowed<Self>; + /// Custom data to store in each identity. type IdtyData: Clone + Codec @@ -453,6 +456,12 @@ pub mod pallet { Error::<T>::OwnerKeyAlreadyUsed ); + // Ensure that the key is not currently as a validator + ensure!( + T::OwnerKeyChangePermission::check_allowed(&idty_index), + Error::<T>::OwnerKeyUsedAsValidator + ); + 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 { @@ -690,6 +699,8 @@ pub mod pallet { AccountNotExist, /// Insufficient balance to create an identity. InsufficientBalance, + /// Owner key currently used as validator. + OwnerKeyUsedAsValidator, } // INTERNAL FUNCTIONS // diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index dcd757c0b..92380ce2c 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -116,6 +116,7 @@ impl pallet_identity::Config for Test { type IdtyNameValidator = IdtyNameValidatorTestImpl; type OnNewIdty = (); type OnRemoveIdty = (); + type OwnerKeyChangePermission = (); type RuntimeEvent = RuntimeEvent; type Signature = Signature; type Signer = AccountPublic; diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs index 73c759cff..8366d1cff 100644 --- a/pallets/identity/src/traits.rs +++ b/pallets/identity/src/traits.rs @@ -94,3 +94,15 @@ impl<AccountId, IdtyIndex> LinkIdty<AccountId, IdtyIndex> for () { Ok(()) } } + +/// Trait for checking whether a key change is allowed for a given identity. +pub trait CheckKeyChangeAllowed<T: Config> { + /// Determines if a key change is allowed for the given identity. + fn check_allowed(account_id: &T::IdtyIndex) -> bool; +} + +impl<T: Config> CheckKeyChangeAllowed<T> for () { + fn check_allowed(_: &T::IdtyIndex) -> bool { + true + } +} diff --git a/pallets/quota/src/mock.rs b/pallets/quota/src/mock.rs index e533345ea..a21f3b85c 100644 --- a/pallets/quota/src/mock.rs +++ b/pallets/quota/src/mock.rs @@ -157,6 +157,7 @@ impl pallet_identity::Config for Test { type IdtyNameValidator = IdtyNameValidatorTestImpl; type OnNewIdty = (); type OnRemoveIdty = (); + type OwnerKeyChangePermission = (); type RuntimeEvent = RuntimeEvent; type Signature = Signature; type Signer = AccountPublic; diff --git a/resources/metadata.scale b/resources/metadata.scale index 7b3fc4e7c424afabf32ed020c7739a4ab0be673f..021ed2e3d4d1432e070d9cfe9600f6fc64df121d 100644 GIT binary patch delta 96 zcmX>;k+X9WXTugog;GYJ?TV$0Kbak4{LAxFi@Z}SLyJ>W9E-yeb23vBOY(~tgji;P qr4+JLD;1JUi;7b7N^&X{N`cB05{nheU<#-6hcGH{x2a_8J`Vt|rz2ee delta 30 mcmeC2#Cd!oXTugog;GY3?TV$0KbfchZemp0URTN3eI5X{y9^@$ diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs index fa247d1c5..cb8a819bb 100644 --- a/runtime/common/src/handlers.rs +++ b/runtime/common/src/handlers.rs @@ -165,3 +165,17 @@ where } } } + +/// Runtime handler OwnerKeyChangePermission. +pub struct OwnerKeyChangePermissionHandler<Runtime>(core::marker::PhantomData<Runtime>); +impl< + Runtime: frame_system::Config + + pallet_identity::Config<IdtyIndex = IdtyIndex> + + pallet_authority_members::Config<MemberId = IdtyIndex>, + > pallet_identity::traits::CheckKeyChangeAllowed<Runtime> + for OwnerKeyChangePermissionHandler<Runtime> +{ + fn check_allowed(idty_index: &IdtyIndex) -> bool { + !pallet_authority_members::Pallet::<Runtime>::online().contains(idty_index) + } +} diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index b376b12d8..3389823fa 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -470,6 +470,7 @@ macro_rules! pallets_config { type IdtyNameValidator = IdtyNameValidatorImpl; type OnNewIdty = OnNewIdtyHandler<Runtime>; type OnRemoveIdty = OnRemoveIdtyHandler<Runtime>; + type OwnerKeyChangePermission = OwnerKeyChangePermissionHandler<Runtime>; type RuntimeEvent = RuntimeEvent; type Signature = Signature; type Signer = <Signature as sp_runtime::traits::Verify>::Signer; diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs index f6bbdf48d..892894e5b 100644 --- a/runtime/gdev/tests/integration_tests.rs +++ b/runtime/gdev/tests/integration_tests.rs @@ -1361,6 +1361,31 @@ fn test_link_account() { }) } +/// test change owner key +#[test] +fn test_change_owner_key_validator_online() { + ExtBuilder::new(1, 3, 4).build().execute_with(|| { + let genesis_hash = System::block_hash(0); + let alice = AccountKeyring::Alice.to_account_id(); + let ferdie = AccountKeyring::Ferdie.to_account_id(); + let payload = (b"icok", genesis_hash, 1u32, alice.clone()).encode(); + let signature = AccountKeyring::Alice.sign(&payload); + + // Alice is an online validator + assert!(pallet_authority_members::OnlineAuthorities::<Runtime>::get().contains(&1)); + + // As an online validator she cannot change key + assert_noop!( + Identity::change_owner_key( + frame_system::RawOrigin::Signed(alice.clone()).into(), + ferdie.clone(), + signature.into() + ), + pallet_identity::Error::<gdev_runtime::Runtime>::OwnerKeyUsedAsValidator + ); + }) +} + /// test change owner key #[test] fn test_change_owner_key() { -- GitLab