diff --git a/pallets/duniter-account/src/lib.rs b/pallets/duniter-account/src/lib.rs index 836220376dd71e95af91c86ae88bcd583115ab46..7244fd3d9bfde32e320bef9f0ed205f2ce58d9ee 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,8 @@ 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) } } @@ -478,16 +487,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/benchmarking.rs b/pallets/identity/src/benchmarking.rs index fea88edfd280be3f36bcee60b0f84a66fc8069e2..fb5950a66fe25045bff9789f760bb3c49bc8bdab 100644 --- a/pallets/identity/src/benchmarking.rs +++ b/pallets/identity/src/benchmarking.rs @@ -254,6 +254,7 @@ benchmarks! { let alice_origin = RawOrigin::Signed(Identities::<T>::get(T::IdtyIndex::one()).unwrap().owner_key); let bob_public = sr25519_generate(0.into(), None); let bob: T::AccountId = MultiSigner::Sr25519(bob_public).into_account().into(); + frame_system::Pallet::<T>::inc_providers(&bob); let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero()); let payload = ( LINK_IDTY_PAYLOAD_PREFIX, genesis_hash, T::IdtyIndex::one(), bob.clone(), diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs index dcbf64d3bfb98d49fca9f38beb911ae7bd9ca835..c89d77aacbcb80a845a9a1f5e96f37f63ea3d867 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,9 +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); + 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()) @@ -584,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()) } @@ -614,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, @@ -624,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 // @@ -773,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 a152bd39c13c00bb67ad08cece0bb4d9d49c1c11..ffc230aa7fdea7ab97547325a2952d23a924294b 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 471c97672353b4b91bb648a14089a582a7c8e9fa..cf6728e0b911d1c7f980cfaf772a968e9d113781 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 fa700844761e49eaa732756378ea805f4f2d9c0e..ef4dce191d85262ad341243a4758665bf623895f 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 @@ -1220,12 +1239,25 @@ fn test_change_owner_key() { let ferdie = AccountKeyring::Ferdie.to_account_id(); let payload = (b"icok", genesis_hash, 4u32, dave.clone()).encode(); let signature = AccountKeyring::Ferdie.sign(&payload); + + assert_eq!( + frame_system::Pallet::<Runtime>::get(&dave).linked_idty, + Some(4) + ); + 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( - frame_system::RawOrigin::Signed(dave).into(), - ferdie, + frame_system::RawOrigin::Signed(dave.clone()).into(), + ferdie.clone(), signature.into() )); + assert_eq!( + frame_system::Pallet::<Runtime>::get(&ferdie).linked_idty, + Some(4) + ); }) } @@ -1306,3 +1338,37 @@ fn smith_data_problem() { run_to_block(4); }); } + +/// test killed account +// The only way to kill an account is to kill the identity +// and transfer all funds. +#[test] +fn test_killed_account() { + ExtBuilder::new(1, 2, 4) + .with_initial_balances(vec![(AccountKeyring::Bob.to_account_id(), 1_000)]) + .build() + .execute_with(|| { + let alice_account = AccountKeyring::Alice.to_account_id(); + let bob_account = AccountKeyring::Bob.to_account_id(); + // check that Alice is 1 and Bob 2 + assert_eq!(Identity::identity_index_of(&alice_account), Some(1)); + assert_eq!(Identity::identity_index_of(&bob_account), Some(2)); + + let _ = Identity::do_remove_identity(2, pallet_identity::RemovalReason::Revoked); + assert_eq!( + frame_system::Pallet::<Runtime>::get(&bob_account).linked_idty, + Some(2) + ); + assert_ok!(Balances::transfer_all( + frame_system::RawOrigin::Signed(bob_account.clone()).into(), + sp_runtime::MultiAddress::Id(alice_account.clone()), + false + )); + + // Bob account should have been reaped + assert_eq!( + frame_system::Pallet::<Runtime>::get(&bob_account), + pallet_duniter_account::AccountData::default() + ); + }) +}