From 3dd7b752f76622bcfd68cd4b1056716732ef8f15 Mon Sep 17 00:00:00 2001 From: Muharem Ismailov Date: Sat, 1 Jul 2023 17:48:53 +0200 Subject: [PATCH] Pallets: Assets `destroy_accounts` releases the deposit (#14443) * assset accounts destroy releases the deposit * enumerate * Update frame/assets/src/functions.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * import defensive from frame_support --------- Co-authored-by: Gavin Wood Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: parity-processbot <> --- frame/assets/src/functions.rs | 23 +++++++++++++++++------ frame/assets/src/tests.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/frame/assets/src/functions.rs b/frame/assets/src/functions.rs index 9c4b1c362e282..c2c1b6839060e 100644 --- a/frame/assets/src/functions.rs +++ b/frame/assets/src/functions.rs @@ -18,7 +18,7 @@ //! Functions for the Assets pallet. use super::*; -use frame_support::{traits::Get, BoundedVec}; +use frame_support::{defensive, traits::Get, BoundedVec}; #[must_use] pub(super) enum DeadConsequence { @@ -760,11 +760,22 @@ impl, I: 'static> Pallet { let mut details = maybe_details.as_mut().ok_or(Error::::Unknown)?; // Should only destroy accounts while the asset is in a destroying state ensure!(details.status == AssetStatus::Destroying, Error::::IncorrectStatus); - - for (who, v) in Account::::drain_prefix(&id) { - let _ = Self::dead_account(&who, &mut details, &v.reason, true); - dead_accounts.push(who); - if dead_accounts.len() >= (max_items as usize) { + for (i, (who, mut v)) in Account::::iter_prefix(&id).enumerate() { + // unreserve the existence deposit if any + if let Some((depositor, deposit)) = v.reason.take_deposit_from() { + T::Currency::unreserve(&depositor, deposit); + } else if let Some(deposit) = v.reason.take_deposit() { + T::Currency::unreserve(&who, deposit); + } + if let Remove = Self::dead_account(&who, &mut details, &v.reason, false) { + Account::::remove(&id, &who); + dead_accounts.push(who); + } else { + // deposit may have been released, need to update `Account` + Account::::insert(&id, &who, v); + defensive!("destroy did not result in dead account?!"); + } + if i + 1 >= (max_items as usize) { break } } diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index 9eb1107aa5209..06d4ec1211737 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -1741,3 +1741,37 @@ fn weights_sane() { let info = crate::Call::::finish_destroy { id: 10 }.get_dispatch_info(); assert_eq!(<() as crate::WeightInfo>::finish_destroy(), info.weight); } + +#[test] +fn asset_destroy_refund_existence_deposit() { + new_test_ext().execute_with(|| { + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, false, 1)); + Balances::make_free_balance_be(&1, 100); + let admin = 1; + let admin_origin = RuntimeOrigin::signed(admin); + + let account2 = 2; // account with own deposit + let account3 = 3; // account with admin's deposit + Balances::make_free_balance_be(&account2, 100); + + assert_eq!(Balances::reserved_balance(&account2), 0); + assert_eq!(Balances::reserved_balance(&account3), 0); + assert_eq!(Balances::reserved_balance(&admin), 0); + + assert_ok!(Assets::touch(RuntimeOrigin::signed(account2), 0)); + assert_ok!(Assets::touch_other(admin_origin.clone(), 0, account3)); + + assert_eq!(Balances::reserved_balance(&account2), 10); + assert_eq!(Balances::reserved_balance(&account3), 0); + assert_eq!(Balances::reserved_balance(&admin), 10); + + assert_ok!(Assets::start_destroy(admin_origin.clone(), 0)); + assert_ok!(Assets::destroy_accounts(admin_origin.clone(), 0)); + assert_ok!(Assets::destroy_approvals(admin_origin.clone(), 0)); + assert_ok!(Assets::finish_destroy(admin_origin.clone(), 0)); + + assert_eq!(Balances::reserved_balance(&account2), 0); + assert_eq!(Balances::reserved_balance(&account3), 0); + assert_eq!(Balances::reserved_balance(&admin), 0); + }); +}