Skip to content

Commit

Permalink
chore: add events for deposit owner changes (#738)
Browse files Browse the repository at this point in the history
Fixes KILTprotocol/ticket#3074.

Cc @kilted-andres 

I tried to keep the event structure in all pallets the same, with the
same name and the same fields `id`, `from` and `to`. The `id` field has
a different type depending on the pallet, but the `from` and `to` are
all the same.
  • Loading branch information
ntn-x2 authored Sep 25, 2024
1 parent 684f2b5 commit 5ef8542
Show file tree
Hide file tree
Showing 19 changed files with 196 additions and 40 deletions.
22 changes: 18 additions & 4 deletions pallets/attestation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,15 @@ pub mod pallet {
/// was deleted.
claim_hash: ClaimHashOf<T>,
},
/// The deposit for an attestation has changed owner.
DepositOwnerChanged {
/// The claim hash of the credential whose deposit owner changed.
id: ClaimHashOf<T>,
/// The old deposit owner.
from: AccountIdOf<T>,
/// The new deposit owner.
to: AccountIdOf<T>,
},
}

#[pallet::error]
Expand Down Expand Up @@ -475,10 +484,15 @@ pub mod pallet {
let attestation = Attestations::<T>::get(claim_hash).ok_or(Error::<T>::NotFound)?;
ensure!(attestation.attester == subject, Error::<T>::NotAuthorized);

AttestationStorageDepositCollector::<T>::change_deposit_owner::<BalanceMigrationManagerOf<T>>(
&claim_hash,
sender,
)?;
let old_deposit_owner = AttestationStorageDepositCollector::<T>::change_deposit_owner::<
BalanceMigrationManagerOf<T>,
>(&claim_hash, sender.clone())?;

Self::deposit_event(Event::<T>::DepositOwnerChanged {
id: claim_hash,
from: old_deposit_owner,
to: sender,
});

Ok(())
}
Expand Down
7 changes: 7 additions & 0 deletions pallets/attestation/src/tests/deposit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ fn test_change_deposit_owner() {
Balances::balance_on_hold(&HoldReason::Deposit.into(), &ACCOUNT_01),
<Test as Config>::Deposit::get()
);
assert!(System::events().iter().any(|e| e.event
== Event::<Test>::DepositOwnerChanged {
id: claim_hash,
from: ACCOUNT_00,
to: ACCOUNT_01
}
.into()));
});
}

Expand Down
25 changes: 21 additions & 4 deletions pallets/delegation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ pub mod pallet {
/// A delegation has been removed.
/// \[remover ID, delegation node ID\]
DelegationRemoved(AccountIdOf<T>, DelegationNodeIdOf<T>),
/// The deposit for a delegation has changed owner.
DepositOwnerChanged {
/// The ID of the delegation whose deposit owner changed.
id: DelegationNodeIdOf<T>,
/// The old deposit owner.
from: AccountIdOf<T>,
/// The new deposit owner.
to: AccountIdOf<T>,
},
}

#[pallet::error]
Expand Down Expand Up @@ -683,17 +692,25 @@ pub mod pallet {
#[pallet::weight(<T as Config>::WeightInfo::change_deposit_owner())]
pub fn change_deposit_owner(origin: OriginFor<T>, delegation_id: DelegationNodeIdOf<T>) -> DispatchResult {
let source = <T as Config>::EnsureOrigin::ensure_origin(origin)?;
let sender = source.sender();

let delegation = DelegationNodes::<T>::get(delegation_id).ok_or(Error::<T>::DelegationNotFound)?;

// Deposit can only be swapped by the owner of the delegation node, not the
// parent or another ancestor.
ensure!(delegation.details.owner == source.subject(), Error::<T>::AccessDenied);

DelegationDepositCollector::<T>::change_deposit_owner::<BalanceMigrationManagerOf<T>>(
&delegation_id,
source.sender(),
)
let old_deposit_owner = DelegationDepositCollector::<T>::change_deposit_owner::<
BalanceMigrationManagerOf<T>,
>(&delegation_id, sender.clone())?;

Self::deposit_event(Event::<T>::DepositOwnerChanged {
id: delegation_id,
from: old_deposit_owner,
to: sender,
});

Ok(())
}

/// Updates the deposit amount to the current deposit rate.
Expand Down
14 changes: 9 additions & 5 deletions pallets/delegation/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ pub(crate) mod runtime {
type AccountId = AccountId;
type Lookup = IdentityLookup<Self::AccountId>;

type RuntimeEvent = ();
type RuntimeEvent = RuntimeEvent;
type BlockHashCount = BlockHashCount;
type DbWeight = RocksDbWeight;
type Version = ();
Expand Down Expand Up @@ -264,7 +264,7 @@ pub(crate) mod runtime {
type MaxFreezes = MaxFreezes;
type Balance = Balance;
type DustRemoval = ();
type RuntimeEvent = ();
type RuntimeEvent = RuntimeEvent;
type ExistentialDeposit = ExistentialDeposit;
type AccountStore = System;
type WeightInfo = ();
Expand All @@ -288,7 +288,7 @@ pub(crate) mod runtime {
type EnsureOrigin = mock_origin::EnsureDoubleOrigin<AccountId, Self::CtypeCreatorId>;
type OriginSuccess = mock_origin::DoubleOrigin<AccountId, Self::CtypeCreatorId>;
type OverarchingOrigin = EnsureSigned<AccountId>;
type RuntimeEvent = ();
type RuntimeEvent = RuntimeEvent;
type WeightInfo = ();

type Currency = Balances;
Expand All @@ -305,7 +305,7 @@ pub(crate) mod runtime {
type EnsureOrigin = mock_origin::EnsureDoubleOrigin<AccountId, DelegatorIdOf<Self>>;
type OriginSuccess = mock_origin::DoubleOrigin<AccountId, DelegatorIdOf<Self>>;
type RuntimeHoldReason = RuntimeHoldReason;
type RuntimeEvent = ();
type RuntimeEvent = RuntimeEvent;
type WeightInfo = ();

type Currency = Balances;
Expand Down Expand Up @@ -335,7 +335,7 @@ pub(crate) mod runtime {
type DelegationNodeId = Hash;
type EnsureOrigin = mock_origin::EnsureDoubleOrigin<AccountId, Self::DelegationEntityId>;
type OriginSuccess = mock_origin::DoubleOrigin<AccountId, Self::DelegationEntityId>;
type RuntimeEvent = ();
type RuntimeEvent = RuntimeEvent;
type MaxSignatureByteLength = MaxSignatureByteLength;
type MaxParentChecks = MaxParentChecks;
type MaxRevocations = MaxRevocations;
Expand Down Expand Up @@ -503,6 +503,10 @@ pub(crate) mod runtime {
let mut ext = sp_io::TestExternalities::new(storage);

ext.execute_with(|| {
// ensure that we are not at the genesis block. Events are not registered for
// the genesis block.
System::set_block_number(System::block_number() + 1);

for (ctype_hash, owner) in self.ctypes.iter() {
ctype::Ctypes::<Test>::insert(
ctype_hash,
Expand Down
9 changes: 8 additions & 1 deletion pallets/delegation/src/tests/deposit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use frame_support::{
use kilt_support::mock::mock_origin::DoubleOrigin;
use sp_runtime::{traits::Zero, TokenError};

use crate::{self as delegation, mock::*, Config, Error, HoldReason};
use crate::{self as delegation, mock::*, Config, Error, Event, HoldReason};

#[test]
fn test_change_deposit_owner() {
Expand Down Expand Up @@ -75,6 +75,13 @@ fn test_change_deposit_owner() {
Balances::balance_on_hold(&HoldReason::Deposit.into(), &ACCOUNT_01),
<Test as Config>::Deposit::get()
);
assert!(System::events().iter().any(|e| e.event
== Event::<Test>::DepositOwnerChanged {
id: delegation_id,
from: ACCOUNT_00,
to: ACCOUNT_01
}
.into()));
});
}

Expand Down
19 changes: 18 additions & 1 deletion pallets/did/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,15 @@ pub mod pallet {
/// A DID-authorised call has been executed.
/// \[DID caller, dispatch result\]
DidCallDispatched(DidIdentifierOf<T>, DispatchResult),
/// The deposit for a DID has changed owner.
DepositOwnerChanged {
/// The DID whose deposit owner changed.
id: DidIdentifierOf<T>,
/// The old deposit owner.
from: AccountIdOf<T>,
/// The new deposit owner.
to: AccountIdOf<T>,
},
}

#[pallet::error]
Expand Down Expand Up @@ -1105,7 +1114,15 @@ pub mod pallet {
let subject = source.subject();
let sender = source.sender();

DidDepositCollector::<T>::change_deposit_owner::<<T as Config>::BalanceMigrationManager>(&subject, sender)?;
let old_deposit_owner = DidDepositCollector::<T>::change_deposit_owner::<
<T as Config>::BalanceMigrationManager,
>(&subject, sender.clone())?;

Self::deposit_event(Event::<T>::DepositOwnerChanged {
id: subject,
from: old_deposit_owner,
to: sender,
});

Ok(())
}
Expand Down
7 changes: 7 additions & 0 deletions pallets/did/src/tests/deposit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,13 @@ fn test_change_deposit_owner() {
Balances::balance_on_hold(&HoldReason::Deposit.into(), &alice_did),
<Test as did::Config>::BaseDeposit::get()
);
assert!(System::events().iter().any(|e| e.event
== Event::<Test>::DepositOwnerChanged {
id: alice_did,
from: ACCOUNT_00,
to: ACCOUNT_01
}
.into()));
});
}

Expand Down
25 changes: 21 additions & 4 deletions pallets/pallet-did-lookup/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,15 @@ pub mod pallet {

/// All AccountIds have been migrated to LinkableAccountId.
MigrationCompleted,
/// The deposit for an linked account has changed owner.
DepositOwnerChanged {
/// The tuple of (DID, linked account) whose deposit owner changed.
id: (DidIdentifierOf<T>, LinkableAccountId),
/// The old deposit owner.
from: AccountIdOf<T>,
/// The new deposit owner.
to: AccountIdOf<T>,
},
}

#[pallet::error]
Expand Down Expand Up @@ -374,14 +383,22 @@ pub mod pallet {
pub fn change_deposit_owner(origin: OriginFor<T>, account: LinkableAccountId) -> DispatchResult {
let source = <T as Config>::EnsureOrigin::ensure_origin(origin)?;
let subject = source.subject();
let sender = source.sender();

let record = ConnectedDids::<T>::get(&account).ok_or(Error::<T>::NotFound)?;
ensure!(record.did == subject, Error::<T>::NotAuthorized);

LinkableAccountDepositCollector::<T>::change_deposit_owner::<BalanceMigrationManagerOf<T>>(
&account,
source.sender(),
)
let old_deposit_owner = LinkableAccountDepositCollector::<T>::change_deposit_owner::<
BalanceMigrationManagerOf<T>,
>(&account, sender.clone())?;

Self::deposit_event(Event::<T>::DepositOwnerChanged {
id: (subject, account),
from: old_deposit_owner,
to: sender,
});

Ok(())
}

/// Updates the deposit amount to the current deposit rate.
Expand Down
4 changes: 4 additions & 0 deletions pallets/pallet-did-lookup/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ impl ExtBuilder {
let mut ext = sp_io::TestExternalities::new(storage);

ext.execute_with(|| {
// ensure that we are not at the genesis block. Events are not registered for
// the genesis block.
System::set_block_number(System::block_number() + 1);

for (sender, did, account) in self.connections {
pallet_did_lookup::Pallet::<Test>::add_association(sender, did, account)
.expect("Should create connection");
Expand Down
9 changes: 8 additions & 1 deletion pallets/pallet-did-lookup/src/tests/deposit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use frame_support::{assert_noop, assert_ok, traits::fungible::InspectHold};
use kilt_support::mock::mock_origin;
use sp_runtime::{traits::Zero, TokenError};

use crate::{mock::*, Error, HoldReason};
use crate::{mock::*, Error, Event, HoldReason};

#[test]
fn test_change_deposit_owner() {
Expand All @@ -39,6 +39,13 @@ fn test_change_deposit_owner() {
Balances::balance_on_hold(&HoldReason::Deposit.into(), &ACCOUNT_01),
<Test as crate::Config>::Deposit::get()
);
assert!(System::events().iter().any(|e| e.event
== Event::<Test>::DepositOwnerChanged {
id: (DID_00, LINKABLE_ACCOUNT_00),
from: ACCOUNT_00,
to: ACCOUNT_01
}
.into()));
})
}

Expand Down
24 changes: 20 additions & 4 deletions pallets/pallet-web3-names/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,15 @@ pub mod pallet {
Web3NameBanned { name: Web3NameOf<T> },
/// A name has been unbanned.
Web3NameUnbanned { name: Web3NameOf<T> },
/// The deposit for a web3name has changed owner.
DepositOwnerChanged {
/// The web3name whose deposit owner changed.
id: Web3NameOf<T>,
/// The old deposit owner.
from: AccountIdOf<T>,
/// The new deposit owner.
to: AccountIdOf<T>,
},
}

#[pallet::error]
Expand Down Expand Up @@ -353,11 +362,18 @@ pub mod pallet {
pub fn change_deposit_owner(origin: OriginFor<T>) -> DispatchResult {
let source = <T as Config>::OwnerOrigin::ensure_origin(origin)?;
let w3n_owner = source.subject();
let sender = source.sender();
let name = Names::<T>::get(&w3n_owner).ok_or(Error::<T>::NotFound)?;
Web3NameStorageDepositCollector::<T>::change_deposit_owner::<BalanceMigrationManagerOf<T>>(
&name,
source.sender(),
)?;

let old_deposit_owner = Web3NameStorageDepositCollector::<T>::change_deposit_owner::<
BalanceMigrationManagerOf<T>,
>(&name, sender.clone())?;

Self::deposit_event(Event::<T>::DepositOwnerChanged {
id: name,
from: old_deposit_owner,
to: sender,
});

Ok(())
}
Expand Down
4 changes: 4 additions & 0 deletions pallets/pallet-web3-names/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ pub(crate) mod runtime {
let mut ext = sp_io::TestExternalities::new(storage);

ext.execute_with(|| {
// ensure that we are not at the genesis block. Events are not registered for
// the genesis block.
System::set_block_number(System::block_number() + 1);

for (owner, web3_name, payer) in self.claimed_web3_names {
pallet_web3_names::Pallet::<Test>::register_name(web3_name, owner, payer)
.expect("Could not register name");
Expand Down
2 changes: 1 addition & 1 deletion pallets/pallet-web3-names/src/tests/claim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn claiming_successful() {
owner_details,
Web3OwnershipOf::<Test> {
owner: DID_00,
claimed_at: 0,
claimed_at: 1,
deposit: Deposit {
owner: ACCOUNT_00,
amount: Web3NameDeposit::get(),
Expand Down
9 changes: 8 additions & 1 deletion pallets/pallet-web3-names/src/tests/deposit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use frame_support::{assert_noop, assert_ok, traits::fungible::InspectHold};
use kilt_support::{mock::mock_origin, Deposit};
use sp_runtime::{traits::Zero, TokenError};

use crate::{mock::*, Config, Error, HoldReason, Owner, Pallet};
use crate::{mock::*, Config, Error, Event, HoldReason, Owner, Pallet};

#[test]
fn test_change_deposit_owner() {
Expand All @@ -48,6 +48,13 @@ fn test_change_deposit_owner() {
Balances::balance_on_hold(&HoldReason::Deposit.into(), &ACCOUNT_01),
<Test as Config>::Deposit::get()
);
assert!(System::events().iter().any(|e| e.event
== Event::<Test>::DepositOwnerChanged {
id: web3_name_00.clone(),
from: ACCOUNT_00,
to: ACCOUNT_01
}
.into()));
})
}

Expand Down
Loading

0 comments on commit 5ef8542

Please sign in to comment.