From 6af881588d7c65889d6f1e6dc514264c6a71ac9c Mon Sep 17 00:00:00 2001 From: bgallois <benjamin@gallois.cc> Date: Fri, 15 Dec 2023 17:41:16 +0100 Subject: [PATCH] fix https://git.duniter.org/nodes/rust/duniter-v2s/-/issues/153 --- pallets/duniter-account/src/lib.rs | 31 ++++++++--------- pallets/identity/src/lib.rs | 23 ++++++------- pallets/identity/src/traits.rs | 6 ++-- runtime/common/src/pallets_config.rs | 2 +- runtime/gdev/tests/integration_tests.rs | 45 ++++++++++++++++++------- 5 files changed, 61 insertions(+), 46 deletions(-) diff --git a/pallets/duniter-account/src/lib.rs b/pallets/duniter-account/src/lib.rs index 836220376..a968968a6 100644 --- a/pallets/duniter-account/src/lib.rs +++ b/pallets/duniter-account/src/lib.rs @@ -32,7 +32,6 @@ use frame_support::pallet_prelude::*; use frame_support::traits::{Currency, ExistenceRequirement, StorageVersion}; use frame_support::traits::{OnUnbalanced, StoredMap}; use frame_system::pallet_prelude::*; -use pallet_identity::IdtyEvent; use pallet_provide_randomness::RequestId; use pallet_quota::traits::RefundFee; use pallet_transaction_payment::OnChargeTransaction; @@ -219,7 +218,16 @@ pub mod pallet { } /// link account to identity - pub fn do_link_identity(account_id: T::AccountId, idty_id: IdtyIdOf<T>) { + pub fn do_link_identity( + account_id: T::AccountId, + idty_id: IdtyIdOf<T>, + ) -> Result<(), DispatchError> { + // Check that account exist + ensure!( + (frame_system::Account::<T>::get(&account_id).providers >= 1) + || (frame_system::Account::<T>::get(&account_id).sufficients >= 1), + pallet_identity::Error::<T>::AccountNotExist + ); // no-op if identity does not change if frame_system::Account::<T>::get(&account_id) .data @@ -234,6 +242,7 @@ pub mod pallet { }); }) } + Ok(()) } } @@ -326,8 +335,9 @@ impl<T> pallet_identity::traits::LinkIdty<T::AccountId, IdtyIdOf<T>> for Pallet< where T: Config, { - fn link_identity(account_id: T::AccountId, idty_id: IdtyIdOf<T>) { - Self::do_link_identity(account_id, idty_id); + fn link_identity(account_id: T::AccountId, idty_id: IdtyIdOf<T>) -> Result<(), DispatchError> { + Self::do_link_identity(account_id, idty_id)?; + Ok(()) } } @@ -478,16 +488,3 @@ where Ok(()) } } - -// implement identity event handler -impl<T: Config> pallet_identity::traits::OnIdtyChange<T> for Pallet<T> { - fn on_idty_change(idty_id: IdtyIdOf<T>, idty_event: &IdtyEvent<T>) { - match idty_event { - // link account to newly created identity - IdtyEvent::Created { owner_key, .. } => { - Self::do_link_identity(owner_key.clone(), idty_id); - } - IdtyEvent::Removed { .. } => {} - } - } -} diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index 388f0d737..c89d77aac 100644 --- a/pallets/identity/src/lib.rs +++ b/pallets/identity/src/lib.rs @@ -328,6 +328,7 @@ pub mod pallet { idty_index, owner_key: owner_key.clone(), }); + T::AccountLinker::link_identity(owner_key.clone(), idty_index)?; T::OnIdtyChange::on_idty_change( idty_index, &IdtyEvent::Created { @@ -454,10 +455,10 @@ pub mod pallet { 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::do_link_account(new_key.clone(), idty_index); + T::AccountLinker::link_identity(new_key.clone(), idty_index)?; Self::deposit_event(Event::IdtyChangedOwnerKey { idty_index, - new_owner_key: new_key.clone(), + new_owner_key: new_key, }); Ok(().into()) @@ -585,7 +586,7 @@ pub mod pallet { Error::<T>::InvalidSignature ); // apply - Self::do_link_account(account_id, idty_index); + T::AccountLinker::link_identity(account_id, idty_index)?; Ok(().into()) } @@ -615,7 +616,7 @@ pub mod pallet { InvalidSignature, /// Invalid revocation key. InvalidRevocationKey, - /// Issuer is not member and can not perform this action + /// Issuer is not member and can not perform this action. IssuerNotMember, /// Identity creation period is not respected. NotRespectIdtyCreationPeriod, @@ -625,12 +626,14 @@ pub mod pallet { OwnerKeyAlreadyUsed, /// Reverting to an old key is prohibited. ProhibitedToRevertToAnOldKey, - /// Already revoked + /// Already revoked. AlreadyRevoked, - /// Can not revoke identity that never was member + /// Can not revoke identity that never was member. CanNotRevokeUnconfirmed, - /// Can not revoke identity that never was member + /// Can not revoke identity that never was member. CanNotRevokeUnvalidated, + /// Cannot link to an inexisting account. + AccountNotExist, } // INTERNAL FUNCTIONS // @@ -774,12 +777,6 @@ pub mod pallet { total_weight.saturating_add(T::WeightInfo::prune_identities_noop()) } - /// link account - fn do_link_account(account_id: T::AccountId, idty_index: T::IdtyIndex) { - // call account linker - T::AccountLinker::link_identity(account_id, idty_index); - } - /// change identity status and reschedule next action fn update_identity_status( idty_index: T::IdtyIndex, diff --git a/pallets/identity/src/traits.rs b/pallets/identity/src/traits.rs index a152bd39c..ffc230aa7 100644 --- a/pallets/identity/src/traits.rs +++ b/pallets/identity/src/traits.rs @@ -53,10 +53,12 @@ impl<T: Config> OnIdtyChange<T> for Tuple { /// trait used to link an account to an identity pub trait LinkIdty<AccountId, IdtyIndex> { - fn link_identity(account_id: AccountId, idty_index: IdtyIndex); + fn link_identity(account_id: AccountId, idty_index: IdtyIndex) -> Result<(), DispatchError>; } impl<AccountId, IdtyIndex> LinkIdty<AccountId, IdtyIndex> for () { - fn link_identity(_: AccountId, _: IdtyIndex) {} + fn link_identity(_: AccountId, _: IdtyIndex) -> Result<(), DispatchError> { + Ok(()) + } } /// trait used only in benchmarks to prepare identity for benchmarking diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index 471c97672..cf6728e0b 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -479,7 +479,7 @@ macro_rules! pallets_config { type IdtyNameValidator = IdtyNameValidatorImpl; type Signer = <Signature as sp_runtime::traits::Verify>::Signer; type Signature = Signature; - type OnIdtyChange = (Wot, SmithSubWot, Quota, Account); + type OnIdtyChange = (Wot, SmithSubWot, Quota); type RuntimeEvent = RuntimeEvent; type WeightInfo = common_runtime::weights::pallet_identity::WeightInfo<Runtime>; #[cfg(feature = "runtime-benchmarks")] diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs index 14ac65f2b..e42ee0a74 100644 --- a/runtime/gdev/tests/integration_tests.rs +++ b/runtime/gdev/tests/integration_tests.rs @@ -1196,19 +1196,38 @@ fn test_oneshot_accounts() { /// test linking account to identity #[test] fn test_link_account() { - 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"link", genesis_hash, 1u32, ferdie.clone()).encode(); - let signature = AccountKeyring::Ferdie.sign(&payload); - // Ferdie's account can be linked to Alice identity - assert_ok!(Identity::link_account( - frame_system::RawOrigin::Signed(alice).into(), - ferdie, - signature.into() - )); - }) + ExtBuilder::new(1, 3, 4) + .with_initial_balances(vec![(AccountKeyring::Alice.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"link", genesis_hash, 1u32, ferdie.clone()).encode(); + let signature = AccountKeyring::Ferdie.sign(&payload); + + // Ferdie's account cannot be linked to Alice identity because the account does not exist + assert_noop!( + Identity::link_account( + frame_system::RawOrigin::Signed(alice.clone()).into(), + ferdie.clone(), + signature.clone().into() + ), + pallet_identity::Error::<gdev_runtime::Runtime>::AccountNotExist + ); + + assert_ok!(Balances::transfer( + frame_system::RawOrigin::Signed(alice.clone()).into(), + MultiAddress::Id(ferdie.clone()), + 1_000 + )); + // Ferdie's account can be linked to Alice identity + assert_ok!(Identity::link_account( + frame_system::RawOrigin::Signed(alice).into(), + ferdie, + signature.into() + )); + }) } /// test change owner key -- GitLab