diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs
index 99babcc97e192f3db0c2d9f6ae3a6bd87e88a704..b6c1bd13c23e32253b38eac3ffbd823bc45ec9ce 100644
--- a/pallets/authority-members/src/lib.rs
+++ b/pallets/authority-members/src/lib.rs
@@ -378,25 +378,25 @@ pub mod pallet {
// Execute the annotated function in a new storage transaction.
// The return type of the annotated function must be `Result`. All changes to storage performed
// by the annotated function are discarded if it returns `Err`, or committed if `Ok`.
- #[frame_support::transactional]
/// Change the owner key of an authority member.
+ #[frame_support::transactional]
pub fn change_owner_key(
member_id: T::MemberId,
new_owner_key: T::AccountId,
) -> DispatchResultWithPostInfo {
- let old_owner_key = Members::<T>::mutate_exists(member_id, |maybe_member_data| {
- if let Some(ref mut member_data) = maybe_member_data {
- Ok(core::mem::replace(
- &mut member_data.owner_key,
- new_owner_key.clone(),
- ))
- } else {
- Err(Error::<T>::MemberNotFound)
- }
+ let old_owner_key = Members::<T>::try_mutate_exists(member_id, |maybe_member_data| {
+ let member_data = maybe_member_data
+ .as_mut()
+ .ok_or(Error::<T>::MemberNotFound)?;
+ Ok::<T::AccountId, Error<T>>(core::mem::replace(
+ &mut member_data.owner_key,
+ new_owner_key.clone(),
+ ))
})?;
let validator_id = T::ValidatorIdOf::convert(old_owner_key.clone())
.ok_or(pallet_session::Error::<T>::NoAssociatedValidatorId)?;
+
let session_keys = pallet_session::NextKeys::<T>::get(validator_id)
.ok_or(Error::<T>::SessionKeysNotProvided)?;
@@ -405,7 +405,7 @@ pub mod pallet {
.dispatch_bypass_filter(frame_system::RawOrigin::Signed(old_owner_key).into())?;
// Set session keys
- let _post_info = pallet_session::Call::<T>::set_keys {
+ pallet_session::Call::<T>::set_keys {
keys: session_keys,
proof: vec![],
}
@@ -491,7 +491,7 @@ pub mod pallet {
.is_ok()
}
- /// C&heck if member is online.
+ /// Check if member is online.
fn is_online(member_id: &T::MemberId) -> bool {
OnlineAuthorities::<T>::get()
.binary_search(member_id)
diff --git a/pallets/distance/src/mock.rs b/pallets/distance/src/mock.rs
index e804df741245c2fc738b79305e15f226033c2333..adb614267dd9e5586d1a9378ce331fbd4ed682b0 100644
--- a/pallets/distance/src/mock.rs
+++ b/pallets/distance/src/mock.rs
@@ -228,9 +228,9 @@ impl pallet_identity::Config for Test {
type IdtyData = ();
type IdtyIndex = u32;
type IdtyNameValidator = IdtyNameValidatorTestImpl;
+ type OnKeyChange = ();
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 a3c550632f5febae809a1bf20cff4c28f75eb579..f0279c36d2886156b12542396d6a09368626b502 100644
--- a/pallets/duniter-wot/src/mock.rs
+++ b/pallets/duniter-wot/src/mock.rs
@@ -107,9 +107,9 @@ impl pallet_identity::Config for Test {
type IdtyData = ();
type IdtyIndex = IdtyIndex;
type IdtyNameValidator = IdtyNameValidatorTestImpl;
+ type OnKeyChange = ();
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 64f47fe83edbed60889149f263e66a6cd14f6afd..5ae90084f4cbcef3d8085e8af0572332fdbcd91d 100644
--- a/pallets/identity/src/lib.rs
+++ b/pallets/identity/src/lib.rs
@@ -134,8 +134,8 @@ 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>;
+ /// A handler called when an identity's owner key change.
+ type OnKeyChange: KeyChange<Self>;
/// Custom data to store in each identity.
type IdtyData: Clone
@@ -456,12 +456,6 @@ 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 {
@@ -503,6 +497,7 @@ pub mod pallet {
IdentityIndexOf::<T>::insert(&idty_value.owner_key, idty_index);
Identities::<T>::insert(idty_index, idty_value);
T::AccountLinker::link_identity(&new_key, idty_index)?;
+ T::OnKeyChange::on_changed(idty_index, new_key.clone())?;
Self::deposit_event(Event::IdtyChangedOwnerKey {
idty_index,
new_owner_key: new_key,
@@ -807,8 +802,6 @@ pub mod pallet {
AccountNotExist,
/// Insufficient balance to create an identity.
InsufficientBalance,
- /// Owner key currently used as validator.
- OwnerKeyUsedAsValidator,
/// Legacy revocation document format is invalid
InvalidLegacyRevocationFormat,
}
diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs
index 70758bc813260c41b9175e9e2225338b89c35c9f..c1b7f7e7b05d1b26e068806c18b49ae629371ad7 100644
--- a/pallets/identity/src/mock.rs
+++ b/pallets/identity/src/mock.rs
@@ -100,9 +100,9 @@ impl pallet_identity::Config for Test {
type IdtyData = ();
type IdtyIndex = u64;
type IdtyNameValidator = IdtyNameValidatorTestImpl;
+ type OnKeyChange = ();
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 8366d1cff76022599feb2d118e622128cc12a59e..9aca1dc64539c05edcd971b9ba07a9737830e5f1 100644
--- a/pallets/identity/src/traits.rs
+++ b/pallets/identity/src/traits.rs
@@ -95,14 +95,14 @@ impl<AccountId, IdtyIndex> LinkIdty<AccountId, IdtyIndex> for () {
}
}
-/// 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;
+/// A trait for handling identity owner key changes.
+pub trait KeyChange<T: Config> {
+ fn on_changed(id: T::IdtyIndex, account_id: T::AccountId) -> Result<(), DispatchError>;
}
-impl<T: Config> CheckKeyChangeAllowed<T> for () {
- fn check_allowed(_: &T::IdtyIndex) -> bool {
- true
+impl<T: Config> KeyChange<T> for () {
+ /// Called when an identity's owner key has changed.
+ fn on_changed(_id: T::IdtyIndex, _account_id: T::AccountId) -> Result<(), DispatchError> {
+ Ok(())
}
}
diff --git a/pallets/quota/src/mock.rs b/pallets/quota/src/mock.rs
index f39b053dfe4c1f565549dd401b54bf52d8b0a1de..070d01d810c03027425d91a283e842e4856e2249 100644
--- a/pallets/quota/src/mock.rs
+++ b/pallets/quota/src/mock.rs
@@ -144,9 +144,9 @@ impl pallet_identity::Config for Test {
type IdtyData = ();
type IdtyIndex = u64;
type IdtyNameValidator = IdtyNameValidatorTestImpl;
+ type OnKeyChange = ();
type OnNewIdty = ();
type OnRemoveIdty = ();
- type OwnerKeyChangePermission = ();
type RuntimeEvent = RuntimeEvent;
type Signature = Signature;
type Signer = AccountPublic;
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index 3fdd7b2d395730e3bb4f45cbb12ae1bc7f3f1817..ba0d0f296b7af1d88ff6f10a5bb9008751bc0aaa 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -171,16 +171,20 @@ where
}
/// Runtime handler OwnerKeyChangePermission.
-pub struct OwnerKeyChangePermissionHandler<Runtime>(core::marker::PhantomData<Runtime>);
+pub struct KeyChangeHandler<Runtime>(core::marker::PhantomData<Runtime>);
impl<
- Runtime: frame_system::Config
+ Runtime: frame_system::Config<AccountId = AccountId>
+ pallet_identity::Config<IdtyIndex = IdtyIndex>
+ pallet_authority_members::Config<MemberId = IdtyIndex>,
- > pallet_identity::traits::CheckKeyChangeAllowed<Runtime>
- for OwnerKeyChangePermissionHandler<Runtime>
+ > pallet_identity::traits::KeyChange<Runtime> for KeyChangeHandler<Runtime>
{
- fn check_allowed(idty_index: &IdtyIndex) -> bool {
- !pallet_authority_members::Pallet::<Runtime>::online().contains(idty_index)
+ fn on_changed(
+ idty_index: IdtyIndex,
+ account_id: AccountId,
+ ) -> Result<(), sp_runtime::DispatchError> {
+ pallet_authority_members::Pallet::<Runtime>::change_owner_key(idty_index, account_id)
+ .map_err(|e| e.error)?;
+ Ok(())
}
}
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index 04fa45d061f2ee8a191a9dd12db531d536707677..c4226e204a6addc561585c69ceddb7bc0b6b710f 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/runtime/common/src/pallets_config.rs
@@ -443,9 +443,9 @@ macro_rules! pallets_config {
type IdtyData = IdtyData;
type IdtyIndex = IdtyIndex;
type IdtyNameValidator = IdtyNameValidatorImpl;
+ type OnKeyChange = KeyChangeHandler<Runtime>;
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 8b859ce71dbf379e7d0621231b36b124824f531e..32dd489f27875fd1f3b3dbce2b25a2bbacd73654 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -1516,154 +1516,139 @@ fn test_link_account() {
})
}
-/// test change owner key
-#[test]
-fn test_change_owner_key_validator_keys_set() {
- ExtBuilder::new(1, 3, 4).build().execute_with(|| {
- let genesis_hash = System::block_hash(0);
- let bob = AccountKeyring::Bob.to_account_id();
- let ferdie = AccountKeyring::Ferdie.to_account_id();
- let payload = (b"icok", genesis_hash, 2u32, bob.clone()).encode();
- let signature = AccountKeyring::Ferdie.sign(&payload);
-
- // Bob is not an online validator, but his identity/owner_key is already associated.
- // He should not be able to change its owner_key.
- assert_ok!(AuthorityMembers::set_session_keys(
- RuntimeOrigin::signed(AccountKeyring::Bob.to_account_id()),
- create_dummy_session_keys()
- ));
- assert!(!pallet_authority_members::OnlineAuthorities::<Runtime>::get().contains(&2));
- assert!(pallet_authority_members::Members::<Runtime>::get(2).is_some());
-
- assert_noop!(
- Identity::change_owner_key(
- RuntimeOrigin::signed(bob.clone()),
- ferdie.clone(),
- signature.into()
- ),
- pallet_identity::Error::<gdev_runtime::Runtime>::OwnerKeyUsedAsValidator
- );
- })
-}
-
/// 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::Ferdie.sign(&payload);
+ ExtBuilder::new(1, 3, 4)
+ .with_initial_balances(vec![(AccountKeyring::Ferdie.to_account_id(), 8888)])
+ .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::Ferdie.sign(&payload);
- // Alice is an online validator
- assert!(pallet_authority_members::OnlineAuthorities::<Runtime>::get().contains(&1));
+ // Alice is an online validator
+ assert!(pallet_authority_members::OnlineAuthorities::<Runtime>::get().contains(&1));
+ assert_eq!(
+ pallet_authority_members::Members::<Runtime>::get(1)
+ .unwrap()
+ .owner_key,
+ alice
+ );
- // As an online validator she cannot change key
- assert_noop!(
- Identity::change_owner_key(
+ assert_ok!(Identity::change_owner_key(
RuntimeOrigin::signed(alice.clone()),
ferdie.clone(),
signature.into()
- ),
- pallet_identity::Error::<gdev_runtime::Runtime>::OwnerKeyUsedAsValidator
- );
- })
+ ),);
+
+ assert_eq!(
+ pallet_authority_members::Members::<Runtime>::get(1)
+ .unwrap()
+ .owner_key,
+ ferdie
+ );
+ })
}
/// test change owner key
#[test]
fn test_change_owner_key() {
- ExtBuilder::new(1, 3, 4).build().execute_with(|| {
- let genesis_hash = System::block_hash(0);
- let charlie = AccountKeyring::Charlie.to_account_id();
- let ferdie = AccountKeyring::Ferdie.to_account_id();
- let payload = (b"icok", genesis_hash, 3u32, charlie.clone()).encode();
- let signature = AccountKeyring::Ferdie.sign(&payload);
-
- SmithMembers::on_smith_goes_offline(3);
- // Charlie is now 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]
- })
- );
+ ExtBuilder::new(1, 3, 4)
+ .with_initial_balances(vec![(AccountKeyring::Ferdie.to_account_id(), 8888)])
+ .build()
+ .execute_with(|| {
+ let genesis_hash = System::block_hash(0);
+ let charlie = AccountKeyring::Charlie.to_account_id();
+ let ferdie = AccountKeyring::Ferdie.to_account_id();
+ let payload = (b"icok", genesis_hash, 3u32, charlie.clone()).encode();
+ let signature = AccountKeyring::Ferdie.sign(&payload);
- assert_eq!(
- frame_system::Pallet::<Runtime>::get(&charlie).linked_idty,
- Some(3)
- );
- assert_eq!(
- frame_system::Pallet::<Runtime>::get(&ferdie).linked_idty,
- None
- );
- // Dave can change his owner key to Ferdie's
- assert_ok!(Identity::change_owner_key(
- RuntimeOrigin::signed(charlie.clone()),
- ferdie.clone(),
- signature.into()
- ));
- assert_eq!(
- frame_system::Pallet::<Runtime>::get(&ferdie).linked_idty,
- Some(3)
- );
+ SmithMembers::on_smith_goes_offline(3);
+ // Charlie is now 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]
+ })
+ );
- // 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]
- })
- );
+ assert_eq!(
+ frame_system::Pallet::<Runtime>::get(&charlie).linked_idty,
+ Some(3)
+ );
+ assert_eq!(
+ frame_system::Pallet::<Runtime>::get(&ferdie).linked_idty,
+ None
+ );
+ // Dave can change his owner key to Ferdie's
+ assert_ok!(Identity::change_owner_key(
+ RuntimeOrigin::signed(charlie.clone()),
+ ferdie.clone(),
+ signature.into()
+ ));
+ assert_eq!(
+ frame_system::Pallet::<Runtime>::get(&ferdie).linked_idty,
+ Some(3)
+ );
- // Ferdie can set its session_keys and go online
- frame_system::Pallet::<Runtime>::inc_providers(&ferdie);
- assert_ok!(AuthorityMembers::set_session_keys(
- RuntimeOrigin::signed(AccountKeyring::Ferdie.to_account_id()),
- create_dummy_session_keys()
- ));
- assert_ok!(AuthorityMembers::go_online(RuntimeOrigin::signed(
- AccountKeyring::Ferdie.to_account_id()
- )));
+ // 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]
+ })
+ );
- // 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(
+ RuntimeOrigin::signed(AccountKeyring::Ferdie.to_account_id()),
+ create_dummy_session_keys()
+ ));
+ assert_ok!(AuthorityMembers::go_online(RuntimeOrigin::signed(
+ AccountKeyring::Ferdie.to_account_id()
+ )));
- run_to_block(25);
+ // 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]
+ })
+ );
- 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] },
- ));
+ run_to_block(25);
- // "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]
- })
- );
- })
+ 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]
+ })
+ );
+ })
}
/// members of the smith subwot can revoke their identity