diff --git a/frame/assets/src/functions.rs b/frame/assets/src/functions.rs index f6e027472f159..47657ff267494 100644 --- a/frame/assets/src/functions.rs +++ b/frame/assets/src/functions.rs @@ -661,8 +661,8 @@ impl, I: 'static> Pallet { status: AssetStatus::Live, }, ); + ensure!(T::CallbackHandle::created(&id, &owner).is_ok(), Error::::CallbackFailed); Self::deposit_event(Event::ForceCreated { asset_id: id, owner: owner.clone() }); - T::CallbackHandle::created(&id, &owner); Ok(()) } @@ -752,7 +752,6 @@ impl, I: 'static> Pallet { approvals_destroyed: removed_approvals as u32, approvals_remaining: details.approvals as u32, }); - T::CallbackHandle::destroyed(&id); Ok(()) })?; Ok(removed_approvals) @@ -767,6 +766,7 @@ impl, I: 'static> Pallet { ensure!(details.status == AssetStatus::Destroying, Error::::IncorrectStatus); ensure!(details.accounts == 0, Error::::InUse); ensure!(details.approvals == 0, Error::::InUse); + ensure!(T::CallbackHandle::destroyed(&id).is_ok(), Error::::CallbackFailed); let metadata = Metadata::::take(&id); T::Currency::unreserve( diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index bb0e8ce8b3704..b36a5cabd377a 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -178,10 +178,14 @@ const LOG_TARGET: &str = "runtime::assets"; /// Trait with callbacks that are executed after successfull asset creation or destruction. pub trait AssetsCallback { /// Indicates that asset with `id` was successfully created by the `owner` - fn created(_id: &AssetId, _owner: &AccountId) {} + fn created(_id: &AssetId, _owner: &AccountId) -> Result<(), ()> { + Ok(()) + } /// Indicates that asset with `id` has just been destroyed - fn destroyed(_id: &AssetId) {} + fn destroyed(_id: &AssetId) -> Result<(), ()> { + Ok(()) + } } /// Empty implementation in case no callbacks are required. @@ -560,6 +564,8 @@ pub mod pallet { IncorrectStatus, /// The asset should be frozen before the given operation. NotFrozen, + /// Callback action resulted in error + CallbackFailed, } #[pallet::call] @@ -618,13 +624,12 @@ pub mod pallet { status: AssetStatus::Live, }, ); - + ensure!(T::CallbackHandle::created(&id, &owner).is_ok(), Error::::CallbackFailed); Self::deposit_event(Event::Created { asset_id: id, creator: owner.clone(), owner: admin, }); - T::CallbackHandle::created(&id, &owner); Ok(()) } diff --git a/frame/assets/src/mock.rs b/frame/assets/src/mock.rs index 14795b10a7e15..3f456a7de3eda 100644 --- a/frame/assets/src/mock.rs +++ b/frame/assets/src/mock.rs @@ -91,12 +91,44 @@ impl pallet_balances::Config for Test { pub struct AssetsCallbackHandle; impl AssetsCallback for AssetsCallbackHandle { - fn created(_id: &AssetId, _owner: &AccountId) { - storage::set(b"asset_created", &().encode()); + fn created(_id: &AssetId, _owner: &AccountId) -> Result<(), ()> { + if Self::should_err() { + Err(()) + } else { + storage::set(Self::CREATED.as_bytes(), &().encode()); + Ok(()) + } } - fn destroyed(_id: &AssetId) { - storage::set(b"asset_destroyed", &().encode()); + fn destroyed(_id: &AssetId) -> Result<(), ()> { + if Self::should_err() { + Err(()) + } else { + storage::set(Self::DESTROYED.as_bytes(), &().encode()); + Ok(()) + } + } +} + +impl AssetsCallbackHandle { + pub const CREATED: &'static str = "asset_created"; + pub const DESTROYED: &'static str = "asset_destroyed"; + + const RETURN_ERROR: &'static str = "return_error"; + + // Configures `Self` to return `Ok` when callbacks are invoked + pub fn set_return_ok() { + storage::clear(Self::RETURN_ERROR.as_bytes()); + } + + // Configures `Self` to return `Err` when callbacks are invoked + pub fn set_return_error() { + storage::set(Self::RETURN_ERROR.as_bytes(), &().encode()); + } + + // If `true`, callback should return `Err`, `Ok` otherwise. + fn should_err() -> bool { + storage::exists(Self::RETURN_ERROR.as_bytes()) } } diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index a2788f7f4a9aa..bc61810a76ac4 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -1261,28 +1261,58 @@ fn querying_roles_should_work() { #[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()); + assert!(storage::get(AssetsCallbackHandle::CREATED.as_bytes()).is_none()); + assert!(storage::get(AssetsCallbackHandle::DESTROYED.as_bytes()).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()); - assert!(storage::get(b"asset_destroyed").is_none()); + assert!(storage::get(AssetsCallbackHandle::CREATED.as_bytes()).is_some()); + assert!(storage::get(AssetsCallbackHandle::DESTROYED.as_bytes()).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)); + // Callback still hasn't been invoked + assert!(storage::get(AssetsCallbackHandle::DESTROYED.as_bytes()).is_none()); + assert_ok!(Assets::finish_destroy(RuntimeOrigin::signed(1), 0)); - assert!(storage::get(b"asset_destroyed").is_some()); + assert!(storage::get(AssetsCallbackHandle::DESTROYED.as_bytes()).is_some()); }); } #[test] fn root_asset_create_should_work() { new_test_ext().execute_with(|| { - assert!(storage::get(b"asset_created").is_none()); + assert!(storage::get(AssetsCallbackHandle::CREATED.as_bytes()).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()); + assert!(storage::get(AssetsCallbackHandle::CREATED.as_bytes()).is_some()); + assert!(storage::get(AssetsCallbackHandle::DESTROYED.as_bytes()).is_none()); + }); +} + +#[test] +fn asset_create_and_destroy_is_reverted_if_callback_fails() { + new_test_ext().execute_with(|| { + // Asset creation fails due to callback failure + AssetsCallbackHandle::set_return_error(); + Balances::make_free_balance_be(&1, 100); + assert_noop!( + Assets::create(RuntimeOrigin::signed(1), 0, 1, 1), + Error::::CallbackFailed + ); + + // Callback succeeds, so asset creation succeeds + AssetsCallbackHandle::set_return_ok(); + assert_ok!(Assets::create(RuntimeOrigin::signed(1), 0, 1, 1)); + + // Asset destroy should fail due to callback failure + AssetsCallbackHandle::set_return_error(); + 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_noop!( + Assets::finish_destroy(RuntimeOrigin::signed(1), 0), + Error::::CallbackFailed + ); }); }