diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index c8f8b4f4373b409d18d490711d4e1ebee3357a8f..77cba1b93f9b73b9a29cfbc3fe717eb5ce09bb2c 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -402,6 +402,7 @@ pub mod pallet { new_key: T::AccountId, new_key_sig: T::NewOwnerKeySignature, ) -> DispatchResultWithPostInfo { + // verification phase let who = ensure_signed(origin)?; let idty_index = @@ -415,12 +416,20 @@ pub mod pallet { ); let block_number = frame_system::Pallet::<T>::block_number(); - if let Some((_, last_change)) = idty_value.old_owner_key { - ensure!( - block_number >= last_change + T::ChangeOwnerKeyPeriod::get(), - Error::<T>::OwnerKeyAlreadyRecentlyChanged - ); - } + let maybe_old_old_owner_key = + if let Some((old_owner_key, last_change)) = idty_value.old_owner_key { + ensure!( + block_number >= last_change + T::ChangeOwnerKeyPeriod::get(), + Error::<T>::OwnerKeyAlreadyRecentlyChanged + ); + ensure!( + old_owner_key != new_key, + Error::<T>::ProhibitedToRevertToAnOldKey + ); + Some(old_owner_key) + } else { + None + }; let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); let new_key_payload = NewOwnerKeyPayload { @@ -435,9 +444,15 @@ pub mod pallet { Error::<T>::InvalidNewOwnerKeySig ); + // Apply phase + if let Some(old_old_owner_key) = maybe_old_old_owner_key { + frame_system::Pallet::<T>::dec_sufficients(&old_old_owner_key); + } IdentityIndexOf::<T>::remove(&idty_value.owner_key); + idty_value.old_owner_key = Some((idty_value.owner_key.clone(), block_number)); idty_value.owner_key = new_key.clone(); + frame_system::Pallet::<T>::inc_sufficients(&idty_value.owner_key); IdentityIndexOf::<T>::insert(&idty_value.owner_key, idty_index); Identities::<T>::insert(idty_index, idty_value); Self::deposit_event(Event::IdtyChangedOwnerKey { @@ -527,6 +542,23 @@ pub mod pallet { Ok(().into()) } + + #[pallet::weight(1_000_000_000)] + pub fn fix_sufficients( + origin: OriginFor<T>, + owner_key: T::AccountId, + inc: bool, + ) -> DispatchResultWithPostInfo { + ensure_root(origin)?; + + if inc { + frame_system::Pallet::<T>::inc_sufficients(&owner_key); + } else { + frame_system::Pallet::<T>::dec_sufficients(&owner_key); + } + + Ok(().into()) + } } // ERRORS // @@ -577,6 +609,8 @@ pub mod pallet { OwnerKeyAlreadyRecentlyChanged, /// Owner key already used OwnerKeyAlreadyUsed, + /// Prohibited to revert to an old key + ProhibitedToRevertToAnOldKey, /// Right already added RightAlreadyAdded, /// Right does not exist @@ -601,6 +635,9 @@ pub mod pallet { // Identity should be removed after the consumers of the identity Identities::<T>::remove(idty_index); frame_system::Pallet::<T>::dec_sufficients(&idty_val.owner_key); + 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 }); T::OnIdtyChange::on_idty_change( idty_index, diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index 082cb28a1daac7024c8b619b5002bd62e8b5670a..38c35851064cde67dc7e56b59ab06a6850ab65fa 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -117,6 +117,7 @@ pub fn new_test_ext(gen_conf: pallet_identity::GenesisConfig<Test>) -> sp_io::Te frame_support::BasicExternalities::execute_with_storage(&mut t, || { // Some dedicated test account + frame_system::Pallet::<Test>::inc_sufficients(&1); frame_system::Pallet::<Test>::inc_providers(&2); frame_system::Pallet::<Test>::inc_providers(&3); }); diff --git a/pallets/identity/src/tests.rs b/pallets/identity/src/tests.rs index be17d906830ef30acfa4dc4e7cb706b14c80da7c..77624e10ca3618d7588bdd20fb5173aa1657ad9f 100644 --- a/pallets/identity/src/tests.rs +++ b/pallets/identity/src/tests.rs @@ -198,7 +198,7 @@ fn test_change_owner_key() { .execute_with(|| { let genesis_hash = System::block_hash(0); let old_owner_key = 1u64; - let new_key_payload = NewOwnerKeyPayload { + let mut new_key_payload = NewOwnerKeyPayload { genesis_hash: &genesis_hash, idty_index: 1u64, old_owner_key: &old_owner_key, @@ -207,7 +207,11 @@ fn test_change_owner_key() { // We need to initialize at least one block before any call run_to_block(1); - // Caller should have an asoociated identity + // Verify genesis data + assert_eq!(System::sufficients(&1), 1); + assert_eq!(System::sufficients(&10), 0); + + // Caller should have an associated identity assert_err!( Identity::change_owner_key( Origin::signed(42), @@ -264,10 +268,15 @@ fn test_change_owner_key() { status: crate::IdtyStatus::Validated, }) ); + // Alice still sufficient + assert_eq!(System::sufficients(&1), 1); + // New owner key should become a sufficient account + assert_eq!(System::sufficients(&10), 1); run_to_block(2); // Alice can't re-change her owner key too early + new_key_payload.old_owner_key = &10; assert_err!( Identity::change_owner_key( Origin::signed(10), @@ -279,6 +288,45 @@ fn test_change_owner_key() { ), Error::<Test>::OwnerKeyAlreadyRecentlyChanged ); + + // Alice can re-change her owner key after ChangeOwnerKeyPeriod blocs + run_to_block(2 + <Test as crate::Config>::ChangeOwnerKeyPeriod::get()); + assert_ok!(Identity::change_owner_key( + Origin::signed(10), + 100, + TestSignature( + 100, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode() + ) + )); + // Old old owner key should not be sufficient anymore + assert_eq!(System::sufficients(&1), 0); + // Old owner key should still sufficient + assert_eq!(System::sufficients(&10), 1); + // New owner key should become a sufficient account + assert_eq!(System::sufficients(&100), 1); + + // Revoke identity 1 + assert_ok!(Identity::revoke_identity( + Origin::signed(42), + 1, + 100, + TestSignature( + 100, + ( + REVOCATION_PAYLOAD_PREFIX, + RevocationPayload { + idty_index: 1u64, + genesis_hash: System::block_hash(0), + } + ) + .encode() + ) + )); + // Old owner key should not be sufficient anymore + assert_eq!(System::sufficients(&10), 0); + // Last owner key should not be sufficient anymore + assert_eq!(System::sufficients(&100), 0); }); } @@ -327,9 +375,17 @@ fn test_idty_revocation() { )); let events = System::events(); - assert_eq!(events.len(), 1); + assert_eq!(events.len(), 2); assert_eq!( events[0], + EventRecord { + phase: Phase::Initialization, + event: Event::System(frame_system::Event::KilledAccount { account: 1 }), + topics: vec![], + } + ); + assert_eq!( + events[1], EventRecord { phase: Phase::Initialization, event: Event::Identity(crate::Event::IdtyRemoved { idty_index: 1 }),