Skip to content

Commit

Permalink
Add missing events to identity pallet (#6261)
Browse files Browse the repository at this point in the history
# Description

E2E tests to Polkadot/Kusama's people chains (in
open-web3-stack/polkadot-ecosystem-tests#63)
revealed that 2 of the identity pallet's extrinsics did not emit events
in case of success:
* `pallet_identity::rename_sub`, and
* `pallet_identity::set_subs`

This PR fixes that.

## Integration

Other than 2 extrinsics emiting an event when previously they did not,
no other behavior in pallets/extrinsics was modified, so no integration
is needed.

## Review Notes

N/A
  • Loading branch information
rockbmb authored Nov 7, 2024
1 parent 566706d commit 65e7972
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
13 changes: 13 additions & 0 deletions prdoc/pr_6261.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Add missing events to identity pallet

doc:
- audience: Runtime Dev
description: |
Extrinsics from `pallet_identity` that were missing an event emission on success now emit one.

crates:
- name: pallet-identity
bump: major
14 changes: 13 additions & 1 deletion substrate/frame/identity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@ pub mod pallet {
RegistrarAdded { registrar_index: RegistrarIndex },
/// A sub-identity was added to an identity and the deposit paid.
SubIdentityAdded { sub: T::AccountId, main: T::AccountId, deposit: BalanceOf<T> },
/// An account's sub-identities were set (in bulk).
SubIdentitiesSet { main: T::AccountId, number_of_subs: u32, new_deposit: BalanceOf<T> },
/// A given sub-account's associated name was changed by its super-identity.
SubIdentityRenamed { sub: T::AccountId, main: T::AccountId },
/// A sub-identity was removed from an identity and the deposit freed.
SubIdentityRemoved { sub: T::AccountId, main: T::AccountId, deposit: BalanceOf<T> },
/// A sub-identity was cleared, and the given deposit repatriated from the
Expand Down Expand Up @@ -591,6 +595,12 @@ pub mod pallet {
SubsOf::<T>::insert(&sender, (new_deposit, ids));
}

Self::deposit_event(Event::SubIdentitiesSet {
main: sender,
number_of_subs: new_subs as u32,
new_deposit,
});

Ok(Some(
T::WeightInfo::set_subs_old(old_ids.len() as u32) // P: Real number of old accounts removed.
// S: New subs added
Expand Down Expand Up @@ -992,7 +1002,9 @@ pub mod pallet {
let sub = T::Lookup::lookup(sub)?;
ensure!(IdentityOf::<T>::contains_key(&sender), Error::<T>::NoIdentity);
ensure!(SuperOf::<T>::get(&sub).map_or(false, |x| x.0 == sender), Error::<T>::NotOwned);
SuperOf::<T>::insert(&sub, (sender, data));
SuperOf::<T>::insert(&sub, (&sender, data));

Self::deposit_event(Event::SubIdentityRenamed { main: sender, sub });
Ok(())
}

Expand Down
11 changes: 11 additions & 0 deletions substrate/frame/identity/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ fn editing_subaccounts_should_work() {

// rename first sub account
assert_ok!(Identity::rename_sub(RuntimeOrigin::signed(ten.clone()), one.clone(), data(11)));
System::assert_last_event(tests::RuntimeEvent::Identity(Event::SubIdentityRenamed {
main: ten.clone(),
sub: one.clone(),
}));
assert_eq!(SuperOf::<Test>::get(one.clone()), Some((ten.clone(), data(11))));
assert_eq!(SuperOf::<Test>::get(two.clone()), Some((ten.clone(), data(2))));
assert_eq!(Balances::free_balance(ten.clone()), 1000 - id_deposit - 2 * sub_deposit);
Expand Down Expand Up @@ -546,6 +550,13 @@ fn setting_subaccounts_should_work() {
assert_ok!(Identity::set_identity(RuntimeOrigin::signed(ten.clone()), Box::new(ten_info)));
assert_eq!(Balances::free_balance(ten.clone()), 1000 - id_deposit);
assert_ok!(Identity::set_subs(RuntimeOrigin::signed(ten.clone()), subs.clone()));

System::assert_last_event(tests::RuntimeEvent::Identity(Event::SubIdentitiesSet {
main: ten.clone(),
number_of_subs: 1,
new_deposit: sub_deposit,
}));

assert_eq!(Balances::free_balance(ten.clone()), 1000 - id_deposit - sub_deposit);
assert_eq!(
SubsOf::<Test>::get(ten.clone()),
Expand Down

0 comments on commit 65e7972

Please sign in to comment.