diff --git a/Cargo.lock b/Cargo.lock index 4756079c630df1a49197085ec29daa6f3dbd1651..5edc011c783706b8d33e370470f4e964c519828d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5234,6 +5234,7 @@ dependencies = [ "serde", "sp-core", "sp-io", + "sp-keystore", "sp-runtime", "sp-std", ] diff --git a/pallets/identity/Cargo.toml b/pallets/identity/Cargo.toml index 5eb5c216737c9c6df5030d9abafe118270591713..8d7f3a9f86142482731d1c1c058f6249b96c7bbb 100644 --- a/pallets/identity/Cargo.toml +++ b/pallets/identity/Cargo.toml @@ -11,7 +11,7 @@ version = '3.0.0' [features] default = ['std'] -runtime-benchmarks = ['frame-benchmarking'] +runtime-benchmarks = ['frame-benchmarking/runtime-benchmarks'] std = [ 'codec/std', 'frame-support/std', @@ -68,6 +68,12 @@ default-features = false git = 'https://github.com/duniter/substrate' branch = 'duniter-substrate-v0.9.32' +[dependencies.sp-io] +default-features = false +git = 'https://github.com/duniter/substrate' +branch = 'duniter-substrate-v0.9.32' + [dev-dependencies] serde = '1.0.119' sp-io = { git = 'https://github.com/duniter/substrate', branch = 'duniter-substrate-v0.9.32' } +sp-keystore = { git = 'https://github.com/duniter/substrate', branch = 'duniter-substrate-v0.9.32' } diff --git a/pallets/identity/src/benchmarking.rs b/pallets/identity/src/benchmarking.rs new file mode 100644 index 0000000000000000000000000000000000000000..ebecf1e33b6b1160a4342db5193390a74f074b7e --- /dev/null +++ b/pallets/identity/src/benchmarking.rs @@ -0,0 +1,296 @@ +// Copyright 2021-2023 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/>. + +#![cfg(feature = "runtime-benchmarks")] + +use super::*; + +//use codec::Encode; +use codec::Encode; +use frame_benchmarking::{account, benchmarks}; +use frame_system::RawOrigin; +use sp_core::Get; +use sp_io::crypto::{sr25519_generate, sr25519_sign}; +use sp_runtime::{AccountId32, MultiSigner}; + +use crate::Pallet; + +const SEED: u32 = 1; + +fn assert_has_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) { + frame_system::Pallet::<T>::assert_has_event(generic_event.into()); +} + +struct Account<T: Config> { + key: T::AccountId, + index: T::IdtyIndex, + origin: <T as frame_system::Config>::RuntimeOrigin, + name: IdtyName, +} + +// Create and confirm one account using Alice authorized account. +// key, origin, name and index are returned. +// Alice next_creatable_identity_on is reinitialized at the end so several account can be +// created in a row. +fn create_one_identity<T: Config>(owner_key: T::AccountId) -> Result<Account<T>, &'static str> { + let caller: T::AccountId = Identities::<T>::get(T::IdtyIndex::one()).unwrap().owner_key; + let caller_origin: <T as frame_system::Config>::RuntimeOrigin = + RawOrigin::Signed(caller.clone()).into(); + let owner_key_origin: <T as frame_system::Config>::RuntimeOrigin = + RawOrigin::Signed(owner_key.clone()).into(); + Pallet::<T>::create_identity(caller_origin.clone(), owner_key.clone())?; + let name = IdtyName("new_identity".into()); + Pallet::<T>::confirm_identity(owner_key_origin.clone(), name.clone())?; + let idty_index = IdentityIndexOf::<T>::get(&owner_key).unwrap(); + // Reset next_creatable_identity_on to add more identities with Alice + <Identities<T>>::mutate_exists(T::IdtyIndex::one(), |idty_val_opt| { + if let Some(ref mut idty_val) = idty_val_opt { + idty_val.next_creatable_identity_on = T::BlockNumber::zero(); + } + }); + Ok(Account { + key: owner_key, + origin: owner_key_origin, + name: name, + index: idty_index, + }) +} + +// Create a dummy identity bypassing all the checks. +fn create_dummy_identity<T: Config>(i: u32) -> Result<(), &'static str> { + let idty_index: T::IdtyIndex = i.into(); + let owner_key: T::AccountId = account("Bob", i, SEED); + let removable_on = T::BlockNumber::zero(); + let value = IdtyValue { + data: Default::default(), + next_creatable_identity_on: T::BlockNumber::zero(), + old_owner_key: None, + owner_key: owner_key.clone(), + removable_on, + status: IdtyStatus::ConfirmedByOwner, + }; + let name = i.to_le_bytes(); + let idty_name = IdtyName(name.into()); + <Identities<T>>::insert(idty_index, value); + IdentitiesRemovableOn::<T>::append(removable_on, (idty_index, IdtyStatus::Created)); + IdentityIndexOf::<T>::insert(owner_key.clone(), idty_index); + <IdentitiesNames<T>>::insert(idty_name.clone(), ()); + Ok(()) +} + +// Add `i` dummy identities. +fn create_identities<T: Config>(i: u32) -> Result<(), &'static str> { + let identities_count = Pallet::<T>::identities_count(); + for j in 0..i { + create_dummy_identity::<T>(j + identities_count + 1)?; + } + assert!( + identities_count + i == Pallet::<T>::identities_count(), + "Identities not created" + ); + Ok(()) +} + +benchmarks! { + where_clause { + where + T::NewOwnerKeySignature: From<sp_core::sr25519::Signature>, + T::RevocationSignature: From<sp_core::sr25519::Signature>, + T::AccountId: From<AccountId32>, + T::IdtyIndex: From<u32>, + } + create_identity { + let caller: T::AccountId = Identities::<T>::get(T::IdtyIndex::one()).unwrap().owner_key; // Alice + let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); + let owner_key: T::AccountId = account("new_identity", 2, SEED); + }: _<T::RuntimeOrigin>(caller_origin.clone(), owner_key.clone()) + verify { + let idty_index = IdentityIndexOf::<T>::get(&owner_key); + assert!(idty_index.is_some(), "Identity not added"); + assert_has_event::<T>(Event::<T>::IdtyCreated { idty_index: idty_index.unwrap(), owner_key: owner_key }.into()); + } + + confirm_identity { + let caller: T::AccountId = Identities::<T>::get(T::IdtyIndex::one()).unwrap().owner_key; + let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); + let owner_key: T::AccountId = account("new_identity", 2, SEED); + let owner_key_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(owner_key.clone()).into(); + Pallet::<T>::create_identity(caller_origin.clone(), owner_key.clone())?; + }: _<T::RuntimeOrigin>(owner_key_origin.clone(), IdtyName("new_identity".into())) + verify { + let idty_index = IdentityIndexOf::<T>::get(&owner_key); + assert_has_event::<T>(Event::<T>::IdtyConfirmed { idty_index: idty_index.unwrap(), owner_key: owner_key, name: IdtyName("new_identity".into()) }.into()); + } + + validate_identity { + // The validation needs to be done on the prepared identity (last genesis identity) in the benchmark genesis. + // This identity is created but not confirmed and have received enough certificates to get a membership. + // The identity needs first to be validated with a call instead of added confirmed at genesis + // to get added to avoid a MembershipRequestNotFound error. + let caller: T::AccountId = Identities::<T>::get(T::IdtyIndex::one()).unwrap().owner_key; + let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); + let index: T::IdtyIndex = NextIdtyIndex::<T>::get() - T::IdtyIndex::one(); + let name = IdtyName("new_identity".into()); + let owner_key: T::AccountId = Identities::<T>::get(index).unwrap().owner_key; + let owner_key_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(owner_key.clone()).into(); + Pallet::<T>::confirm_identity(owner_key_origin.clone(), name.clone())?; + }: _<T::RuntimeOrigin>(caller_origin, index) + verify { + assert_has_event::<T>(Event::<T>::IdtyValidated { idty_index: index }.into()); + } + + change_owner_key { + let old_key: T::AccountId = account("new_identity", 2, SEED); + let account: Account<T> = create_one_identity(old_key.clone())?; + + // Change key a first time to add an old-old key + let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); + let new_key_payload = NewOwnerKeyPayload { + genesis_hash: &genesis_hash, + idty_index: account.index, + old_owner_key: &account.key, + }; + let message = (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode(); + let caller_public = sr25519_generate(0.into(), None); + let caller: T::AccountId = MultiSigner::Sr25519(caller_public.clone()).into_account().into(); + let signature = sr25519_sign(0.into(), &caller_public, &message).unwrap().into(); + Pallet::<T>::change_owner_key(account.origin.clone(), caller.clone(), signature)?; + + // Change key a second time to benchmark + // The sufficients for the old_old key will drop to 0 during benchmark + let caller_origin: <T as frame_system::Config>::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); + let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); + let new_key_payload = NewOwnerKeyPayload { + genesis_hash: &genesis_hash, + idty_index: account.index, + old_owner_key: &caller_public, + }; + let message = (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode(); + let caller_public = sr25519_generate(0.into(), None); + let caller: T::AccountId = MultiSigner::Sr25519(caller_public.clone()).into_account().into(); + let signature = sr25519_sign(0.into(), &caller_public, &message).unwrap().into(); + <frame_system::Pallet<T>>::set_block_number(<frame_system::Pallet<T>>::block_number() + T::ChangeOwnerKeyPeriod::get()); + }: _<T::RuntimeOrigin>(caller_origin.clone(), caller.clone(), signature) + verify { + assert_has_event::<T>(Event::<T>::IdtyChangedOwnerKey { idty_index: account.index, new_owner_key: caller.clone() }.into()); + assert!(IdentityIndexOf::<T>::get(&caller).unwrap() == account.index, "Owner key not changed"); + } + + revoke_identity { + let old_key: T::AccountId = account("new_identity", 2, SEED); + let account: Account<T> = create_one_identity(old_key.clone())?; + + // Change key + // The sufficients for the old key will drop to 0 during benchmark + let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); + let new_key_payload = NewOwnerKeyPayload { + genesis_hash: &genesis_hash, + idty_index: account.index, + old_owner_key: &account.key, + }; + let message = (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode(); + let caller_public = sr25519_generate(0.into(), None); + let caller: T::AccountId = MultiSigner::Sr25519(caller_public.clone()).into_account().into(); + let signature = sr25519_sign(0.into(), &caller_public, &message).unwrap().into(); + Pallet::<T>::change_owner_key(account.origin.clone(), caller.clone(), signature)?; + + let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); + let revocation_payload = RevocationPayload { + genesis_hash: &genesis_hash, + idty_index: account.index.clone(), + }; + let message = (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode(); + let signature = sr25519_sign(0.into(), &caller_public, &message).unwrap().into(); + }: _<T::RuntimeOrigin>(account.origin.clone(), account.index.clone().into(), caller.clone(), signature) + verify { + assert_has_event::<T>(Event::<T>::IdtyRemoved { idty_index: account.index }.into()); + assert!(IdentityIndexOf::<T>::get(&account.key).is_none(), "Identity not revoked"); + } + + remove_identity { + let new_identity: T::AccountId = account("new_identity", 2, SEED); + let account: Account<T> = create_one_identity(new_identity)?; + let identities = Pallet::<T>::identities_count(); + }: _<T::RuntimeOrigin>(RawOrigin::Root.into(), account.index.clone(),Some(account.name.clone())) + verify { + assert!( + Pallet::<T>::identities_count() == identities - 1, + "Identities not removed" + ); + assert_has_event::<T>(Event::<T>::IdtyRemoved { idty_index: account.index }.into()); + assert!(IdentityIndexOf::<T>::get(&account.key).is_none(), "Identity not removed"); + } + + // The complexity depends on the number of identities to prune + prune_item_identities_names { + // Populate identities + let identities_count = Pallet::<T>::identities_count(); + let i in 1 .. 1000 => create_identities::<T>(i)?; + + let mut names = Vec::<IdtyName>::new(); + for k in 1..i { + let name: IdtyName = IdtyName((k + identities_count).to_le_bytes().into()); + assert!(IdentitiesNames::<T>::contains_key(&name), "Name not existing"); + names.push(name); + } + }: _<T::RuntimeOrigin>(RawOrigin::Root.into(), names.clone()) + verify { + for name in names { + assert!(!IdentitiesNames::<T>::contains_key(&name), "Name existing"); + } + } + + fix_sufficients { + let new_identity: T::AccountId = account("Bob", 2, SEED); + let account: Account<T> = create_one_identity(new_identity)?; + let sufficient = frame_system::Pallet::<T>::sufficients(&account.key); + }: _<T::RuntimeOrigin>(RawOrigin::Root.into(), account.key.clone(), true) + verify { + assert!(sufficient < frame_system::Pallet::<T>::sufficients(&account.key), "Sufficient not incremented"); + } + + impl_benchmark_test_suite!( + Pallet, + // Create genesis identity Alice to test benchmark in mock + crate::mock::new_test_ext(crate::mock::IdentityConfig{ identities: vec![ + GenesisIdty { + index: 1, + name: IdtyName::from("Alice"), + value: IdtyValue { + data: (), + next_creatable_identity_on: 0, + old_owner_key: None, + owner_key: account("Alice", 1, SEED), + removable_on: 0, + status: crate::IdtyStatus::Validated, + }, + }, + GenesisIdty { + index: 2, + name: IdtyName::from("Bob"), + value: IdtyValue { + data: (), + next_creatable_identity_on: 0, + old_owner_key: None, + owner_key: account("Bob", 1, SEED), + removable_on: 0, + status: crate::IdtyStatus::Created, + }, + }, + ]}), + crate::mock::Test, + ); +} diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index ab95a439ddeefeb46628214c20d2c67e1bffa638..4d371afaf122e33e0e9529fb572ebfc32801f96a 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2021 Axiom-Team +// Copyright 2021-2023 Axiom-Team // // This file is part of Duniter-v2S. // @@ -26,8 +26,8 @@ mod mock; #[cfg(test)] mod tests; -/*#[cfg(feature = "runtime-benchmarks")] -mod benchmarking;*/ +#[cfg(feature = "runtime-benchmarks")] +pub mod benchmarking; pub use pallet::*; pub use types::*; diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs index 53b8f4ec35c27c5f4ac2749326b4244c948a36ab..9556974392d736e30576b21574395dfe18658c44 100644 --- a/pallets/identity/src/mock.rs +++ b/pallets/identity/src/mock.rs @@ -21,15 +21,25 @@ use frame_support::{ traits::{Everything, GenesisBuild, OnFinalize, OnInitialize}, }; use frame_system as system; -use sp_core::H256; +use sp_core::{Pair, H256}; +use sp_keystore::{testing::KeyStore, KeystoreExt}; use sp_runtime::{ - testing::{Header, TestSignature, UintAuthorityId}, + testing::Header, traits::{BlakeTwo256, IdentityLookup}, + MultiSignature, MultiSigner, }; +use std::sync::Arc; -type AccountId = u64; type Block = frame_system::mocking::MockBlock<Test>; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>; +pub type Signature = MultiSignature; +pub type AccountPublic = <Signature as Verify>::Signer; +pub type AccountId = <AccountPublic as IdentifyAccount>::AccountId; + +fn account(id: u8) -> AccountId { + let pair = sp_core::sr25519::Pair::from_seed(&[id; 32]); + MultiSigner::Sr25519(pair.public()).into_account() +} // Configure a mock runtime to test the pallet. frame_support::construct_runtime!( @@ -98,12 +108,12 @@ impl pallet_identity::Config for Test { type IdtyData = (); type IdtyNameValidator = IdtyNameValidatorTestImpl; type IdtyIndex = u64; - type NewOwnerKeySigner = UintAuthorityId; - type NewOwnerKeySignature = TestSignature; + type NewOwnerKeySigner = AccountPublic; + type NewOwnerKeySignature = Signature; type OnIdtyChange = (); type RemoveIdentityConsumers = (); - type RevocationSigner = UintAuthorityId; - type RevocationSignature = TestSignature; + type RevocationSigner = AccountPublic; + type RevocationSignature = Signature; type RuntimeEvent = RuntimeEvent; } @@ -117,12 +127,16 @@ 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); + frame_system::Pallet::<Test>::inc_sufficients(&account(1)); + frame_system::Pallet::<Test>::inc_providers(&account(2)); + frame_system::Pallet::<Test>::inc_providers(&account(3)); }); - sp_io::TestExternalities::new(t) + let keystore = KeyStore::new(); + let mut ext = sp_io::TestExternalities::new(t); + ext.register_extension(KeystoreExt(Arc::new(keystore))); + ext.execute_with(|| System::set_block_number(1)); + ext } pub fn run_to_block(n: u64) { diff --git a/pallets/identity/src/tests.rs b/pallets/identity/src/tests.rs index ea495479017617d449f012a2fc7e680cdb0e23bc..f04d9d7cb9065be4d5602715021ea48d7c930fe4 100644 --- a/pallets/identity/src/tests.rs +++ b/pallets/identity/src/tests.rs @@ -21,9 +21,31 @@ use crate::{ }; use codec::Encode; use frame_support::{assert_noop, assert_ok}; -use sp_runtime::testing::TestSignature; +use sp_core::sr25519::Pair as KeyPair; +use sp_core::Pair; +use sp_runtime::{traits::IdentifyAccount, MultiSignature, MultiSigner}; -type IdtyVal = IdtyValue<u64, u64, ()>; +type IdtyVal = IdtyValue<u64, AccountId, ()>; + +// Store the account id and the key pair to sign payload +struct Account { + id: AccountId, + signer: KeyPair, +} + +// Create an Account given a u8 +fn account(id: u8) -> Account { + let pair = sp_core::sr25519::Pair::from_seed(&[id; 32]); + Account { + id: MultiSigner::Sr25519(pair.public()).into_account(), + signer: pair, + } +} + +// Sign a payload using a key pair +fn test_signature(key_pair: KeyPair, payload: Vec<u8>) -> MultiSignature { + MultiSignature::Sr25519(key_pair.sign(&payload)) +} fn alice() -> GenesisIdty<Test> { GenesisIdty { @@ -33,7 +55,7 @@ fn alice() -> GenesisIdty<Test> { data: (), next_creatable_identity_on: 0, old_owner_key: None, - owner_key: 1, + owner_key: account(1).id, removable_on: 0, status: crate::IdtyStatus::Validated, }, @@ -48,7 +70,7 @@ fn bob() -> GenesisIdty<Test> { data: (), next_creatable_identity_on: 0, old_owner_key: None, - owner_key: 2, + owner_key: account(2).id, removable_on: 0, status: crate::IdtyStatus::Validated, }, @@ -75,11 +97,14 @@ fn test_create_identity_ok() { run_to_block(1); // Alice should be able to create an identity - assert_ok!(Identity::create_identity(RuntimeOrigin::signed(1), 2)); + assert_ok!(Identity::create_identity( + RuntimeOrigin::signed(account(1).id), + account(2).id + )); System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated { idty_index: 2, - owner_key: 2, + owner_key: account(2).id, })); }); } @@ -94,7 +119,10 @@ fn test_create_identity_but_not_confirm_it() { run_to_block(1); // Alice should be able to create an identity - assert_ok!(Identity::create_identity(RuntimeOrigin::signed(1), 2)); + assert_ok!(Identity::create_identity( + RuntimeOrigin::signed(account(1).id), + account(2).id + )); // The identity shoud expire in blocs #3 run_to_block(3); @@ -105,11 +133,14 @@ fn test_create_identity_but_not_confirm_it() { // We shoud be able to recreate the identity run_to_block(4); - assert_ok!(Identity::create_identity(RuntimeOrigin::signed(1), 2)); + assert_ok!(Identity::create_identity( + RuntimeOrigin::signed(account(1).id), + account(2).id + )); System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated { idty_index: 3, - owner_key: 2, + owner_key: account(2).id, })); }); } @@ -124,11 +155,14 @@ fn test_idty_creation_period() { run_to_block(1); // Alice should be able to create an identity - assert_ok!(Identity::create_identity(RuntimeOrigin::signed(1), 2)); + assert_ok!(Identity::create_identity( + RuntimeOrigin::signed(account(1).id), + account(2).id + )); System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated { idty_index: 2, - owner_key: 2, + owner_key: account(2).id, })); assert_eq!(Identity::identity(1).unwrap().next_creatable_identity_on, 4); @@ -136,21 +170,25 @@ fn test_idty_creation_period() { // Alice cannot create a new identity before block #4 run_to_block(2); assert_eq!( - Identity::create_identity(RuntimeOrigin::signed(1), 3), + Identity::create_identity(RuntimeOrigin::signed(account(1).id), account(3).id), Err(Error::<Test>::NotRespectIdtyCreationPeriod.into()) ); // Alice should be able to create a second identity after block #4 run_to_block(4); - assert_ok!(Identity::create_identity(RuntimeOrigin::signed(1), 3)); + assert_ok!(Identity::create_identity( + RuntimeOrigin::signed(account(1).id), + account(3).id + )); System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyCreated { idty_index: 3, - owner_key: 3, + owner_key: account(3).id, })); }); } +// #[test] fn test_change_owner_key() { new_test_ext(IdentityConfig { @@ -158,7 +196,7 @@ fn test_change_owner_key() { }) .execute_with(|| { let genesis_hash = System::block_hash(0); - let old_owner_key = 1u64; + let old_owner_key = account(1).id; let mut new_key_payload = NewOwnerKeyPayload { genesis_hash: &genesis_hash, idty_index: 1u64, @@ -169,25 +207,29 @@ fn test_change_owner_key() { run_to_block(1); // Verify genesis data - assert_eq!(System::sufficients(&1), 1); - assert_eq!(System::sufficients(&10), 0); - + assert_eq!(System::sufficients(&account(1).id), 1); + assert_eq!(System::sufficients(&account(10).id), 0); // Caller should have an associated identity assert_noop!( Identity::change_owner_key( - RuntimeOrigin::signed(42), - 10, - TestSignature(10, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + RuntimeOrigin::signed(account(42).id), + account(10).id, + test_signature( + account(10).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload.clone()).encode() + ) ), Error::<Test>::IdtyIndexNotFound ); - // Payload must be signed by the new key assert_noop!( Identity::change_owner_key( - RuntimeOrigin::signed(1), - 10, - TestSignature(42, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + RuntimeOrigin::signed(account(1).id), + account(10).id, + test_signature( + account(42).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload.clone()).encode() + ) ), Error::<Test>::InvalidNewOwnerKeySig ); @@ -195,9 +237,9 @@ fn test_change_owner_key() { // Payload must be prefixed assert_noop!( Identity::change_owner_key( - RuntimeOrigin::signed(1), - 10, - TestSignature(10, new_key_payload.encode()) + RuntimeOrigin::signed(account(1).id), + account(10).id, + test_signature(account(10).signer, new_key_payload.clone().encode()) ), Error::<Test>::InvalidNewOwnerKeySig ); @@ -205,46 +247,53 @@ fn test_change_owner_key() { // New owner key should not be used by another identity assert_noop!( Identity::change_owner_key( - RuntimeOrigin::signed(1), - 2, - TestSignature(2, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + RuntimeOrigin::signed(account(1).id), + account(2).id, + test_signature( + account(2).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload.clone()).encode() + ) ), Error::<Test>::OwnerKeyAlreadyUsed ); // Alice can change her owner key assert_ok!(Identity::change_owner_key( - RuntimeOrigin::signed(1), - 10, - TestSignature(10, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + RuntimeOrigin::signed(account(1).id), + account(10).id, + test_signature( + account(10).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload.clone()).encode() + ) )); assert_eq!( Identity::identity(1), Some(IdtyVal { data: (), next_creatable_identity_on: 0, - old_owner_key: Some((1, 1)), - owner_key: 10, + old_owner_key: Some((account(1).id, 1)), + owner_key: account(10).id, removable_on: 0, status: crate::IdtyStatus::Validated, }) ); // Alice still sufficient - assert_eq!(System::sufficients(&1), 1); + assert_eq!(System::sufficients(&account(1).id), 1); // New owner key should become a sufficient account - assert_eq!(System::sufficients(&10), 1); + assert_eq!(System::sufficients(&account(10).id), 1); run_to_block(2); - + // // Alice can't re-change her owner key too early - new_key_payload.old_owner_key = &10; + let old = account(10).id; + new_key_payload.old_owner_key = &old; assert_noop!( Identity::change_owner_key( - RuntimeOrigin::signed(10), - 100, - TestSignature( - 100, - (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode() + RuntimeOrigin::signed(account(10).id), + account(100).id, + test_signature( + account(100).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload.clone()).encode() ) ), Error::<Test>::OwnerKeyAlreadyRecentlyChanged @@ -253,27 +302,27 @@ fn test_change_owner_key() { // 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( - RuntimeOrigin::signed(10), - 100, - TestSignature( - 100, - (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode() + RuntimeOrigin::signed(account(10).id), + account(100).id, + test_signature( + account(100).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload.clone()).encode() ) )); // Old old owner key should not be sufficient anymore - assert_eq!(System::sufficients(&1), 0); + assert_eq!(System::sufficients(&account(1).id), 0); // Old owner key should still sufficient - assert_eq!(System::sufficients(&10), 1); + assert_eq!(System::sufficients(&account(10).id), 1); // New owner key should become a sufficient account - assert_eq!(System::sufficients(&100), 1); + assert_eq!(System::sufficients(&account(100).id), 1); // Revoke identity 1 assert_ok!(Identity::revoke_identity( - RuntimeOrigin::signed(42), + RuntimeOrigin::signed(account(42).id), 1, - 100, - TestSignature( - 100, + account(100).id, + test_signature( + account(100).signer, ( REVOCATION_PAYLOAD_PREFIX, RevocationPayload { @@ -285,9 +334,9 @@ fn test_change_owner_key() { ) )); // Old owner key should not be sufficient anymore - assert_eq!(System::sufficients(&10), 0); + assert_eq!(System::sufficients(&account(10).id), 0); // Last owner key should not be sufficient anymore - assert_eq!(System::sufficients(&100), 0); + assert_eq!(System::sufficients(&account(100).id), 0); }); } @@ -301,7 +350,7 @@ fn test_idty_revocation_with_old_key() { let new_key_payload = NewOwnerKeyPayload { genesis_hash: &genesis_hash, idty_index: 1u64, - old_owner_key: &1u64, + old_owner_key: &account(1).id, }; let revocation_payload = RevocationPayload { idty_index: 1u64, @@ -313,22 +362,28 @@ fn test_idty_revocation_with_old_key() { // Change alice owner key assert_ok!(Identity::change_owner_key( - RuntimeOrigin::signed(1), - 10, - TestSignature(10, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + RuntimeOrigin::signed(account(1).id), + account(10).id, + test_signature( + account(10).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode() + ) )); assert!(Identity::identity(&1).is_some()); let idty_val = Identity::identity(&1).unwrap(); - assert_eq!(idty_val.owner_key, 10); - assert_eq!(idty_val.old_owner_key, Some((1, 1))); + assert_eq!(idty_val.owner_key, account(10).id); + assert_eq!(idty_val.old_owner_key, Some((account(1).id, 1))); // We should be able to revoke Alice identity with old key run_to_block(2); assert_ok!(Identity::revoke_identity( - RuntimeOrigin::signed(42), + RuntimeOrigin::signed(account(42).id), 1, - 1, - TestSignature(1, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) + account(1).id, + test_signature( + account(1).signer, + (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode() + ) )); //run_to_block(2 + <Test as crate::Config>::ChangeOwnerKeyPeriod::get()); @@ -345,7 +400,7 @@ fn test_idty_revocation_with_old_key_after_old_key_expiration() { let new_key_payload = NewOwnerKeyPayload { genesis_hash: &genesis_hash, idty_index: 1u64, - old_owner_key: &1u64, + old_owner_key: &account(1).id, }; let revocation_payload = RevocationPayload { idty_index: 1u64, @@ -357,23 +412,29 @@ fn test_idty_revocation_with_old_key_after_old_key_expiration() { // Change alice owner key assert_ok!(Identity::change_owner_key( - RuntimeOrigin::signed(1), - 10, - TestSignature(10, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode()) + RuntimeOrigin::signed(account(1).id), + account(10).id, + test_signature( + account(10).signer, + (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode() + ) )); assert!(Identity::identity(&1).is_some()); let idty_val = Identity::identity(&1).unwrap(); - assert_eq!(idty_val.owner_key, 10); - assert_eq!(idty_val.old_owner_key, Some((1, 1))); + assert_eq!(idty_val.owner_key, account(10).id); + assert_eq!(idty_val.old_owner_key, Some((account(1).id, 1))); // We should not be able to revoke Alice identity with old key after ChangeOwnerKeyPeriod run_to_block(2 + <Test as crate::Config>::ChangeOwnerKeyPeriod::get()); assert_noop!( Identity::revoke_identity( - RuntimeOrigin::signed(42), + RuntimeOrigin::signed(account(42).id), 1, - 1, - TestSignature(1, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) + account(1).id, + test_signature( + account(1).signer, + (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode() + ) ), Error::<Test>::InvalidRevocationKey ); @@ -397,10 +458,13 @@ fn test_idty_revocation() { // Payload must be signed by the right identity assert_eq!( Identity::revoke_identity( - RuntimeOrigin::signed(1), + RuntimeOrigin::signed(account(1).id), 1, - 42, - TestSignature(42, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) + account(42).id, + test_signature( + account(42).signer, + (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode() + ) ), Err(Error::<Test>::InvalidRevocationKey.into()) ); @@ -408,24 +472,27 @@ fn test_idty_revocation() { // Payload must be prefixed assert_eq!( Identity::revoke_identity( - RuntimeOrigin::signed(1), - 1, + RuntimeOrigin::signed(account(1).id), 1, - TestSignature(1, revocation_payload.encode()) + account(1).id, + test_signature(account(1).signer, revocation_payload.encode()) ), Err(Error::<Test>::InvalidRevocationSig.into()) ); // Anyone can submit a revocation payload assert_ok!(Identity::revoke_identity( - RuntimeOrigin::signed(42), + RuntimeOrigin::signed(account(42).id), 1, - 1, - TestSignature(1, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) + account(1).id, + test_signature( + account(1).signer, + (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode() + ) )); System::assert_has_event(RuntimeEvent::System(frame_system::Event::KilledAccount { - account: 1, + account: account(1).id, })); System::assert_has_event(RuntimeEvent::Identity(crate::Event::IdtyRemoved { idty_index: 1, @@ -436,10 +503,13 @@ fn test_idty_revocation() { // The identity no longer exists assert_eq!( Identity::revoke_identity( - RuntimeOrigin::signed(1), + RuntimeOrigin::signed(account(1).id), 1, - 1, - TestSignature(1, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode()) + account(1).id, + test_signature( + account(1).signer, + (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode() + ) ), Err(Error::<Test>::IdtyNotFound.into()) ); diff --git a/runtime/common/src/entities.rs b/runtime/common/src/entities.rs index a81dac008f1c1bed1abe5c797bef7b38a98967ff..81a35891149971b5e38f726d3470f3ba478f4627 100644 --- a/runtime/common/src/entities.rs +++ b/runtime/common/src/entities.rs @@ -99,6 +99,12 @@ impl sp_runtime::traits::Verify for NewOwnerKeySignature { } } +impl From<sp_core::sr25519::Signature> for NewOwnerKeySignature { + fn from(a: sp_core::sr25519::Signature) -> Self { + NewOwnerKeySignature(a) + } +} + #[cfg_attr(feature = "std", derive(Deserialize, Serialize))] #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo)] pub struct SmithMembershipMetaData<SessionKeysWrapper> { diff --git a/runtime/gdev/src/lib.rs b/runtime/gdev/src/lib.rs index eb15c38e00925f24dba59de5d664d2554330b674..554d71af989d7def73f5db9cb9627b90f6622b1e 100644 --- a/runtime/gdev/src/lib.rs +++ b/runtime/gdev/src/lib.rs @@ -146,6 +146,7 @@ mod benches { [pallet_provide_randomness, ProvideRandomness] [pallet_upgrade_origin, UpgradeOrigin] [pallet_duniter_account, Account] + [pallet_identity, Identity] // Substrate [pallet_balances, Balances] [frame_benchmarking::baseline, Baseline::<Runtime>]