From 8ad9cf5522d91661e5207fd9c8833b6dd4c42d8d Mon Sep 17 00:00:00 2001
From: Dinonard <dino.pacandi@gmail.com>
Date: Wed, 19 Oct 2022 13:24:53 +0200
Subject: [PATCH] Address review comments

---
 frame/assets/src/functions.rs |  4 ++--
 frame/assets/src/lib.rs       |  8 +++++++-
 frame/assets/src/mock.rs      | 21 ++++++++++++++++++---
 frame/assets/src/tests.rs     | 27 +++++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/frame/assets/src/functions.rs b/frame/assets/src/functions.rs
index 953949e80b270..870413b4717bd 100644
--- a/frame/assets/src/functions.rs
+++ b/frame/assets/src/functions.rs
@@ -654,8 +654,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				is_frozen: false,
 			},
 		);
+		Self::deposit_event(Event::ForceCreated { asset_id: id, owner: owner.clone() });
 		T::CallbackHandle::created(&id, &owner);
-		Self::deposit_event(Event::ForceCreated { asset_id: id, owner });
 		Ok(())
 	}
 
@@ -704,8 +704,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 					T::Currency::unreserve(&owner, approval.deposit);
 				}
 
+				Self::deposit_event(Event::Destroyed { asset_id: id.clone() });
 				T::CallbackHandle::destroyed(&id);
-				Self::deposit_event(Event::Destroyed { asset_id: id });
 
 				Ok(DestroyWitness {
 					accounts: details.accounts,
diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs
index a9b011f537735..38bfa6599c710 100644
--- a/frame/assets/src/lib.rs
+++ b/frame/assets/src/lib.rs
@@ -553,8 +553,14 @@ pub mod pallet {
 					is_frozen: false,
 				},
 			);
+
+			Self::deposit_event(Event::Created {
+				asset_id: id,
+				creator: owner.clone(),
+				owner: admin,
+			});
 			T::CallbackHandle::created(&id, &owner);
-			Self::deposit_event(Event::Created { asset_id: id, creator: owner, owner: admin });
+
 			Ok(())
 		}
 
diff --git a/frame/assets/src/mock.rs b/frame/assets/src/mock.rs
index 4e23cb59da2f5..e4d1a03ab3160 100644
--- a/frame/assets/src/mock.rs
+++ b/frame/assets/src/mock.rs
@@ -20,6 +20,7 @@
 use super::*;
 use crate as pallet_assets;
 
+use codec::Encode;
 use frame_support::{
 	construct_runtime, parameter_types,
 	traits::{ConstU32, ConstU64, GenesisBuild},
@@ -45,6 +46,9 @@ construct_runtime!(
 	}
 );
 
+type AccountId = u64;
+type AssetId = u32;
+
 impl frame_system::Config for Test {
 	type BaseCallFilter = frame_support::traits::Everything;
 	type BlockWeights = ();
@@ -55,7 +59,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;
@@ -84,10 +88,21 @@ 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) {
+		sp_io::storage::set(b"asset_created", &true.encode());
+	}
+
+	fn destroyed(_id: &AssetId) {
+		sp_io::storage::set(b"asset_destroyed", &true.encode());
+	}
+}
+
 impl Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type Balance = u64;
-	type AssetId = u32;
+	type AssetId = AssetId;
 	type Currency = Balances;
 	type ForceOrigin = frame_system::EnsureRoot<u64>;
 	type AssetDeposit = ConstU64<1>;
@@ -98,7 +113,7 @@ impl Config for Test {
 	type StringLimit = ConstU32<50>;
 	type Freezer = TestFreezer;
 	type WeightInfo = ();
-	type CallbackHandle = ();
+	type CallbackHandle = AssetsCallbackHandle;
 	type Extra = ();
 }
 
diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs
index 598b6049b3d57..92ea28807d687 100644
--- a/frame/assets/src/tests.rs
+++ b/frame/assets/src/tests.rs
@@ -999,3 +999,30 @@ 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!(sp_io::storage::get(b"asset_created").is_none());
+		assert!(sp_io::storage::get(b"asset_destroyed").is_none());
+
+		Balances::make_free_balance_be(&1, 100);
+		assert_ok!(Assets::create(Origin::signed(1), 0, 1, 1));
+		assert!(sp_io::storage::get(b"asset_created").is_some());
+		assert!(sp_io::storage::get(b"asset_destroyed").is_none());
+
+		let w = Asset::<Test>::get(0).unwrap().destroy_witness();
+		assert_ok!(Assets::destroy(Origin::signed(1), 0, w));
+		assert!(sp_io::storage::get(b"asset_destroyed").is_some());
+	});
+}
+
+#[test]
+fn root_asset_create_should_work() {
+	new_test_ext().execute_with(|| {
+		assert!(sp_io::storage::get(b"asset_created").is_none());
+		assert_ok!(Assets::force_create(Origin::root(), 0, 1, true, 1));
+		assert!(sp_io::storage::get(b"asset_created").is_some());
+		assert!(sp_io::storage::get(b"asset_destroyed").is_none());
+	});
+}