Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add CallbackHandle to pallet-assets #12307

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,7 @@ impl pallet_assets::Config for Runtime {
type StringLimit = StringLimit;
type Freezer = ();
type Extra = ();
type CallbackHandle = ();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Sorry am late to the party.), was just wondering if OnCreated = (); OnDestroyed = () might make it a bit more similar to other pallet callbacks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea was to keep it simple by only having one type but I do like your suggestion. It's more verbose :)

type WeightInfo = pallet_assets::weights::SubstrateWeight<Runtime>;
type RemoveItemsLimit = ConstU32<1000>;
#[cfg(feature = "runtime-benchmarks")]
Expand Down
4 changes: 3 additions & 1 deletion frame/assets/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
status: AssetStatus::Live,
},
);
Self::deposit_event(Event::ForceCreated { asset_id: id, owner });
Self::deposit_event(Event::ForceCreated { asset_id: id, owner: owner.clone() });
T::CallbackHandle::created(&id, &owner);
Ok(())
}

Expand Down Expand Up @@ -754,6 +755,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
approvals_destroyed: removed_approvals as u32,
approvals_remaining: details.approvals as u32,
});
T::CallbackHandle::destroyed(&id);
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
})?;
Ok(removed_approvals)
Expand Down
29 changes: 28 additions & 1 deletion frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@
//!
//! Please refer to the [`Pallet`] struct for details on publicly available functions.
//!
//! ### Callbacks
//!
//! Using `CallbackHandle` associated type, user can configure custom callback functions which are
//! executed when new asset is created or an existing asset is destroyed.
//!
//! ## Related Modules
//!
//! * [`System`](../frame_system/index.html)
Expand Down Expand Up @@ -171,6 +176,18 @@ pub use weights::WeightInfo;

type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;

/// Trait with callbacks that are executed after successfull asset creation or destruction.
pub trait AssetsCallback<AssetId, AccountId> {
/// Indicates that asset with `id` was successfully created by the `owner`
fn created(_id: &AssetId, _owner: &AccountId) {}

/// Indicates that asset with `id` has just been destroyed
fn destroyed(_id: &AssetId) {}
}

/// Empty implementation in case no callbacks are required.
impl<AssetId, AccountId> AssetsCallback<AssetId, AccountId> for () {}

#[frame_support::pallet]
pub mod pallet {
use super::*;
Expand Down Expand Up @@ -283,6 +300,9 @@ pub mod pallet {
/// Additional data to be stored with an account's asset balance.
type Extra: Member + Parameter + Default + MaxEncodedLen;

/// Callback methods for asset state change (e.g. asset created or destroyed)
type CallbackHandle: AssetsCallback<Self::AssetId, Self::AccountId>;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;

Expand Down Expand Up @@ -598,7 +618,14 @@ pub mod pallet {
status: AssetStatus::Live,
},
);
Self::deposit_event(Event::Created { asset_id: id, creator: owner, owner: admin });

Self::deposit_event(Event::Created {
asset_id: id,
creator: owner.clone(),
owner: admin,
});
T::CallbackHandle::created(&id, &owner);

Ok(())
}

Expand Down
19 changes: 18 additions & 1 deletion frame/assets/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
use super::*;
use crate as pallet_assets;

use codec::Encode;
use frame_support::{
construct_runtime, parameter_types,
traits::{AsEnsureOriginWithArg, ConstU32, ConstU64, GenesisBuild},
};
use sp_core::H256;
use sp_io::storage;
use sp_runtime::{
testing::Header,
traits::{BlakeTwo256, IdentityLookup},
Expand All @@ -45,6 +47,9 @@ construct_runtime!(
}
);

type AccountId = u64;
type AssetId = u32;

impl frame_system::Config for Test {
type BaseCallFilter = frame_support::traits::Everything;
type BlockWeights = ();
Expand All @@ -55,7 +60,7 @@ impl frame_system::Config for Test {
type BlockNumber = u64;
type Hash = H256;
type Hashing = BlakeTwo256;
type AccountId = u64;
type AccountId = AccountId;
type Lookup = IdentityLookup<Self::AccountId>;
type Header = Header;
type RuntimeEvent = RuntimeEvent;
Expand Down Expand Up @@ -84,6 +89,17 @@ impl pallet_balances::Config for Test {
type ReserveIdentifier = [u8; 8];
}

pub struct AssetsCallbackHandle;
impl AssetsCallback<AssetId, AccountId> for AssetsCallbackHandle {
fn created(_id: &AssetId, _owner: &AccountId) {
storage::set(b"asset_created", &().encode());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can setup some helpers in your mock.rs for this:

frame_support::parameter_types! {
	pub storage AssetCreated: bool = false;
	pub storage AssetDestroyed: bool = false;
}

And then use them like AssetCreated::{get, set}. They are also automatically reset by new_test_ext.

}

fn destroyed(_id: &AssetId) {
storage::set(b"asset_destroyed", &().encode());
}
}

impl Config for Test {
type RuntimeEvent = RuntimeEvent;
type Balance = u64;
Expand All @@ -100,6 +116,7 @@ impl Config for Test {
type StringLimit = ConstU32<50>;
type Freezer = TestFreezer;
type WeightInfo = ();
type CallbackHandle = AssetsCallbackHandle;
type Extra = ();
type RemoveItemsLimit = ConstU32<5>;
#[cfg(feature = "runtime-benchmarks")]
Expand Down
30 changes: 30 additions & 0 deletions frame/assets/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use frame_support::{
traits::{fungibles::InspectEnumerable, Currency},
};
use pallet_balances::Error as BalancesError;
use sp_io::storage;
use sp_runtime::{traits::ConvertInto, TokenError};

fn asset_ids() -> Vec<u32> {
Expand Down Expand Up @@ -1194,3 +1195,32 @@ fn querying_roles_should_work() {
assert_eq!(Assets::freezer(0), Some(4));
});
}

#[test]
fn normal_asset_create_and_destroy_callbacks_should_work() {
new_test_ext().execute_with(|| {
assert!(storage::get(b"asset_created").is_none());
assert!(storage::get(b"asset_destroyed").is_none());

Balances::make_free_balance_be(&1, 100);
assert_ok!(Assets::create(RuntimeOrigin::signed(1), 0, 1, 1));
assert!(storage::get(b"asset_created").is_some());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe check that is actually true, and not just that it exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do that, the nature of the check is that it's None vs Some.
In that regard, perhaps just changing the type to () would be better.

assert!(storage::get(b"asset_destroyed").is_none());

assert_ok!(Assets::start_destroy(RuntimeOrigin::signed(1), 0));
assert_ok!(Assets::destroy_accounts(RuntimeOrigin::signed(1), 0));
assert_ok!(Assets::destroy_approvals(RuntimeOrigin::signed(1), 0));
assert_ok!(Assets::finish_destroy(RuntimeOrigin::signed(1), 0));
assert!(storage::get(b"asset_destroyed").is_some());
});
}

#[test]
fn root_asset_create_should_work() {
new_test_ext().execute_with(|| {
assert!(storage::get(b"asset_created").is_none());
assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
assert!(storage::get(b"asset_created").is_some());
assert!(storage::get(b"asset_destroyed").is_none());
});
}
1 change: 1 addition & 0 deletions frame/transaction-payment/asset-tx-payment/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ impl pallet_assets::Config for Runtime {
type StringLimit = ConstU32<20>;
type Freezer = ();
type Extra = ();
type CallbackHandle = ();
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
type WeightInfo = ();
type RemoveItemsLimit = ConstU32<1000>;
pallet_assets::runtime_benchmarks_enabled! {
Expand Down